Removed special unique exception logic.

zeroc-ice · Dec 4, 2024 · d0f5768 · d0f5768
1 parent 6179dde
commit d0f5768
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 35 deletions.
diff --git a/cpp/src/Slice/MetadataValidation.cpp b/cpp/src/Slice/MetadataValidation.cpp
@@ -19,13 +19,12 @@ namespace
     public:
         MetadataVisitor(string_view language, map<string, MetadataInfo> knownMetadata);
 
-        // We don't visit `ClassDef` and `InterfaceDef` because their metadata is always stored on their corresponding
-        // `ClassDecl` and `InterfaceDecl` types. So just visiting the `Decl` types is sufficient to check everything.
-
         bool visitUnitStart(const UnitPtr&) final;
         bool visitModuleStart(const ModulePtr&) final;
         void visitClassDecl(const ClassDeclPtr&) final;
+        bool visitClassDefStart(const ClassDefPtr&) final;
         void visitInterfaceDecl(const InterfaceDeclPtr&) final;
+        bool visitInterfaceDefStart(const InterfaceDefPtr&) final;
         bool visitExceptionStart(const ExceptionPtr&) final;
         bool visitStructStart(const StructPtr&) final;
         void visitOperation(const OperationPtr&) final;
@@ -41,7 +40,7 @@ namespace
 
         string_view _language;
         map<string, MetadataInfo> _knownMetadata;
-        map<string, MetadataPtr> _seenDirectives;
+        set<string> _seenDirectives;
     };
 }
 
@@ -170,12 +169,24 @@ MetadataVisitor::visitClassDecl(const ClassDeclPtr& p)
     p->setMetadata(validate(p->getMetadata(), p));
 }
 
+bool
+MetadataVisitor::visitClassDefStart(const ClassDefPtr& p)
+{
+    return true;
+}
+
 void
 MetadataVisitor::visitInterfaceDecl(const InterfaceDeclPtr& p)
 {
     p->setMetadata(validate(p->getMetadata(), p));
 }
 
+bool
+MetadataVisitor::visitInterfaceDefStart(const InterfaceDefPtr& p)
+{
+    return true;
+}
+
 bool
 MetadataVisitor::visitExceptionStart(const ExceptionPtr& p)
 {
@@ -426,34 +437,13 @@ MetadataVisitor::isMetadataValid(const MetadataPtr& metadata, const SyntaxTreeBa
     // Fifth we check if this metadata is a duplicate, i.e. have we already seen this metadata in this context?
     if (info.mustBeUnique)
     {
-        // 'emplace' only returns `true` if the value wasn't already present in the set.
-        bool wasInserted = _seenDirectives.emplace(directive, metadata).second;
+        // 'insert' only returns `true` if the value wasn't already present in the set.
+        bool wasInserted = _seenDirectives.insert(directive).second;
         if (!wasInserted)
         {
-            // We make a special exception to uniqueness for metadata on forward declarations, since there can be
-            // multiple forward declarations for the same entity, but they all share a single backing `MetadataList`.
-            // So for these types, even 'unique' metadata to appear multiple times, but we require them to be identical.
-            if (dynamic_pointer_cast<ClassDecl>(p) || dynamic_pointer_cast<InterfaceDecl>(p))
-            {
-                const MetadataPtr& previousMetadata = _seenDirectives[directive];
-                if (arguments != previousMetadata->arguments())
-                {
-                    ostringstream msg;
-                    msg << "ignoring duplicate metadata: '" << *metadata
-                        << "' does not match previously applied metadata of '" << *previousMetadata << "'";
-                    p->unit()->warning(metadata->file(), metadata->line(), InvalidMetadata, msg.str());
-                }
-
-                // Even in this special case, we want to remove the metadata. At worst it was invalid, and at best,
-                // it's an exact duplicate of some other metadata. Either way, there's no reason to keep it around.
-                isValid = false;
-            }
-            else
-            {
-                auto msg = "ignoring duplicate metadata: '" + directive + "' has already been applied in this context";
-                p->unit()->warning(metadata->file(), metadata->line(), InvalidMetadata, msg);
-                isValid = false;
-            }
+            auto msg = "ignoring duplicate metadata: '" + directive + "' has already been applied in this context";
+            p->unit()->warning(metadata->file(), metadata->line(), InvalidMetadata, msg);
+            isValid = false;
         }
     }
 

diff --git a/cpp/src/Slice/Parser.cpp b/cpp/src/Slice/Parser.cpp
@@ -251,7 +251,7 @@ void
 Slice::DefinitionContext::initSuppressedWarnings()
 {
     _suppressedWarnings.clear();
-    for (const auto& metadata : getMetadata())
+    for (const auto& metadata : _metadata)
     {
         if (metadata->directive() != "suppress-warning")
         {

diff --git a/cpp/src/Slice/Parser.h b/cpp/src/Slice/Parser.h
@@ -572,8 +572,6 @@ namespace Slice
         int compactId() const;
         std::string kindOf() const final;
 
-        // Class metadata is always stored on the underlying decl type, not the definition.
-        // So we override these `xMetadata` functions to forward to `_declarations->xMetadata()` instead.
         MetadataList getMetadata() const final;
         void setMetadata(MetadataList metadata) final;
 
@@ -710,8 +708,6 @@ namespace Slice
         // Returns the type IDs of all the interfaces in the inheritance tree, in alphabetical order.
         StringList ids() const;
 
-        // Interface metadata is always stored on the underlying decl type, not the definition.
-        // So we override these `xMetadata` functions to forward to `_declarations->xMetadata()` instead.
         MetadataList getMetadata() const final;
         void setMetadata(MetadataList metadata) final;