[clang] a65d530 - [ODRHash] Detect duplicate `ObjCProtocolDecl` ODR mismatches during parsing.

Volodymyr Sapsai via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 17 18:32:23 PST 2022


Author: Volodymyr Sapsai
Date: 2022-11-17T18:31:32-08:00
New Revision: a65d5309d5b73527efcbdec49a3ba9bba0fd873d

URL: https://github.com/llvm/llvm-project/commit/a65d5309d5b73527efcbdec49a3ba9bba0fd873d
DIFF: https://github.com/llvm/llvm-project/commit/a65d5309d5b73527efcbdec49a3ba9bba0fd873d.diff

LOG: [ODRHash] Detect duplicate `ObjCProtocolDecl` ODR mismatches during parsing.

When during parsing we encountered a duplicate `ObjCProtocolDecl`, we
were always emitting an error. With this change we accept
* when a previous `ObjCProtocolDecl` is in a hidden [sub]module;
* parsed `ObjCProtocolDecl` is the same as the previous one.

And in case of mismatches we provide more detailed error messages.

rdar://93069080

Differential Revision: https://reviews.llvm.org/D130327

Added: 
    clang/test/Modules/hidden-duplicates.m

Modified: 
    clang/include/clang/AST/DeclObjC.h
    clang/include/clang/AST/ODRDiagsEmitter.h
    clang/include/clang/Basic/DiagnosticASTKinds.td
    clang/include/clang/Sema/Sema.h
    clang/lib/AST/DeclObjC.cpp
    clang/lib/AST/ODRDiagsEmitter.cpp
    clang/lib/Parse/ParseObjc.cpp
    clang/lib/Sema/SemaDeclObjC.cpp
    clang/test/Modules/compare-objc-protocol.m

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/DeclObjC.h b/clang/include/clang/AST/DeclObjC.h
index 3d20d172dc631..6e83060d5a4d4 100644
--- a/clang/include/clang/AST/DeclObjC.h
+++ b/clang/include/clang/AST/DeclObjC.h
@@ -2230,6 +2230,13 @@ class ObjCProtocolDecl : public ObjCContainerDecl,
   /// Starts the definition of this Objective-C protocol.
   void startDefinition();
 
+  /// Starts the definition without sharing it with other redeclarations.
+  /// Such definition shouldn't be used for anything but only to compare if
+  /// a duplicate is compatible with previous definition or if it is
+  /// a distinct duplicate.
+  void startDuplicateDefinitionForComparison();
+  void mergeDuplicateDefinitionWithCommon(const ObjCProtocolDecl *Definition);
+
   /// Produce a name to be used for protocol's metadata. It comes either via
   /// objc_runtime_name attribute or protocol name.
   StringRef getObjCRuntimeNameAsString() const;

diff  --git a/clang/include/clang/AST/ODRDiagsEmitter.h b/clang/include/clang/AST/ODRDiagsEmitter.h
index 4c41a4a6004f1..d3b7f5e6b4109 100644
--- a/clang/include/clang/AST/ODRDiagsEmitter.h
+++ b/clang/include/clang/AST/ODRDiagsEmitter.h
@@ -55,6 +55,16 @@ class ODRDiagsEmitter {
       const ObjCProtocolDecl *SecondProtocol,
       const struct ObjCProtocolDecl::DefinitionData *SecondDD) const;
 
+  /// Diagnose ODR mismatch between ObjCProtocolDecl with 
diff erent definitions.
+  bool diagnoseMismatch(const ObjCProtocolDecl *FirstProtocol,
+                        const ObjCProtocolDecl *SecondProtocol) const {
+    assert(FirstProtocol->data().Definition !=
+               SecondProtocol->data().Definition &&
+           "Don't diagnose 
diff erences when definitions are merged already");
+    return diagnoseMismatch(FirstProtocol, SecondProtocol,
+                            &SecondProtocol->data());
+  }
+
   /// Get the best name we know for the module that owns the given
   /// declaration, or an empty string if the declaration is not from a module.
   static std::string getOwningModuleNameForDiagnostic(const Decl *D);

diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 0f7e256d8056f..15bd9d7c0e49a 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -622,10 +622,11 @@ def err_module_odr_violation_mismatch_decl : Error<
   "%select{end of class|public access specifier|private access specifier|"
   "protected access specifier|static assert|field|method|type alias|typedef|"
   "data member|friend declaration|function template|method|property}3">;
-def note_module_odr_violation_mismatch_decl : Note<"but in '%0' found "
+def note_module_odr_violation_mismatch_decl : Note<
+  "but in %select{'%1'|definition here}0 found "
   "%select{end of class|public access specifier|private access specifier|"
   "protected access specifier|static assert|field|method|type alias|typedef|"
-  "data member|friend declaration|function template|method|property}1">;
+  "data member|friend declaration|function template|method|property}2">;
 
 def err_module_odr_violation_record : Error<
   "%q0 has 
diff erent definitions in 
diff erent modules; first 
diff erence is "
@@ -843,11 +844,12 @@ def err_module_odr_violation_referenced_protocols : Error <
   "%4 referenced %plural{1:protocol|:protocols}4|"
   "%ordinal4 referenced protocol with name %5"
   "}3">;
-def note_module_odr_violation_referenced_protocols : Note <"but in '%0' found "
+def note_module_odr_violation_referenced_protocols : Note <
+  "but in %select{'%1'|definition here}0 found "
   "%select{"
-  "%2 referenced %plural{1:protocol|:protocols}2|"
-  "%ordinal2 referenced protocol with 
diff erent name %3"
-  "}1">;
+  "%3 referenced %plural{1:protocol|:protocols}3|"
+  "%ordinal3 referenced protocol with 
diff erent name %4"
+  "}2">;
 
 def err_module_odr_violation_objc_method : Error<
   "%q0 has 
diff erent definitions in 
diff erent modules; first 
diff erence is "
@@ -860,15 +862,16 @@ def err_module_odr_violation_objc_method : Error<
   "%select{regular|direct}5 method %4|"
   "method %4"
   "}3">;
-def note_module_odr_violation_objc_method : Note<"but in '%0' found "
+def note_module_odr_violation_objc_method : Note<
+  "but in %select{'%1'|definition here}0 found "
   "%select{"
-  "method %2 with 
diff erent return type %3|"
-  "method %2 as %select{class|instance}3 method|"
-  "%select{no|'required'|'optional'}2 method control|"
-  "method %2 with %select{no designated initializer|designated initializer}3|"
-  "%select{regular|direct}3 method %2|"
-  "
diff erent method %2"
-  "}1">;
+  "method %3 with 
diff erent return type %4|"
+  "method %3 as %select{class|instance}4 method|"
+  "%select{no|'required'|'optional'}3 method control|"
+  "method %3 with %select{no designated initializer|designated initializer}4|"
+  "%select{regular|direct}4 method %3|"
+  "
diff erent method %3"
+  "}2">;
 
 def err_module_odr_violation_method_params : Error<
   "%q0 has 
diff erent definitions in 
diff erent modules; first 
diff erence is "
@@ -881,15 +884,16 @@ def err_module_odr_violation_method_params : Error<
   "%select{method %5|constructor|destructor}4 "
     "with %ordinal6 parameter named %7"
   "}3">;
-def note_module_odr_violation_method_params : Note<"but in '%0' found "
+def note_module_odr_violation_method_params : Note<
+  "but in %select{'%1'|definition here}0 found "
   "%select{"
-  "%select{method %3|constructor|destructor}2 "
-    "that has %4 parameter%s4|"
-  "%select{method %3|constructor|destructor}2 "
-    "with %ordinal4 parameter of type %5%select{| decayed from %7}6|"
-  "%select{method %3|constructor|destructor}2 "
-    "with %ordinal4 parameter named %5"
-  "}1">;
+  "%select{method %4|constructor|destructor}3 "
+    "that has %5 parameter%s5|"
+  "%select{method %4|constructor|destructor}3 "
+    "with %ordinal5 parameter of type %6%select{| decayed from %8}7|"
+  "%select{method %4|constructor|destructor}3 "
+    "with %ordinal5 parameter named %6"
+  "}2">;
 
 def err_module_odr_violation_objc_property : Error<
   "%q0 has 
diff erent definitions in 
diff erent modules; first 
diff erence is "
@@ -903,15 +907,15 @@ def err_module_odr_violation_objc_property : Error<
     "unsafe_unretained|nullability|null_resettable|class|direct}5' attribute"
   "}3">;
 def note_module_odr_violation_objc_property : Note<
-  "but in '%0' found "
+  "but in %select{'%1'|definition here}0 found "
   "%select{"
-  "property %2|"
-  "property %2 with type %3|"
-  "%select{no|'required'|'optional'}2 property control|"
-  "property %2 with 
diff erent '%select{none|readonly|getter|assign|"
+  "property %3|"
+  "property %3 with type %4|"
+  "%select{no|'required'|'optional'}3 property control|"
+  "property %3 with 
diff erent '%select{none|readonly|getter|assign|"
     "readwrite|retain|copy|nonatomic|setter|atomic|weak|strong|"
-    "unsafe_unretained|nullability|null_resettable|class|direct}3' attribute"
-  "}1">;
+    "unsafe_unretained|nullability|null_resettable|class|direct}4' attribute"
+  "}2">;
 
 def err_module_odr_violation_mismatch_decl_unknown : Error<
   "%q0 %select{with definition in module '%2'|defined here}1 has 
diff erent "
@@ -920,11 +924,11 @@ def err_module_odr_violation_mismatch_decl_unknown : Error<
   "friend declaration|function template|method|"
   "property|unexpected decl}3">;
 def note_module_odr_violation_mismatch_decl_unknown : Note<
-  "but in '%0' found "
+  "but in %select{'%1'|definition here}0 found "
   "%select{||||
diff erent static assert|
diff erent field|
diff erent method|"
   "
diff erent type alias|
diff erent typedef|
diff erent data member|"
   "
diff erent friend declaration|
diff erent function template|
diff erent method|"
-  "
diff erent property|another unexpected decl}1">;
+  "
diff erent property|another unexpected decl}2">;
 
 
 def remark_sanitize_address_insert_extra_padding_accepted : Remark<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 4250d38869ddc..4696403d6f0fe 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3411,6 +3411,20 @@ class Sema final {
   /// in case of a structural mismatch.
   bool ActOnDuplicateDefinition(Decl *Prev, SkipBodyInfo &SkipBody);
 
+  /// Check ODR hashes for C/ObjC when merging types from modules.
+  /// Differently from C++, actually parse the body and reject in case
+  /// of a mismatch.
+  template <typename T,
+            typename = std::enable_if_t<std::is_base_of<NamedDecl, T>::value>>
+  bool ActOnDuplicateODRHashDefinition(T *Duplicate, T *Previous) {
+    if (Duplicate->getODRHash() != Previous->getODRHash())
+      return false;
+
+    // Make the previous decl visible.
+    makeMergedDefinitionVisible(Previous);
+    return true;
+  }
+
   typedef void *SkippedDefinitionContext;
 
   /// Invoked when we enter a tag definition that we're skipping.
@@ -10118,7 +10132,8 @@ class Sema final {
       SourceLocation AtProtoInterfaceLoc, IdentifierInfo *ProtocolName,
       SourceLocation ProtocolLoc, Decl *const *ProtoRefNames,
       unsigned NumProtoRefs, const SourceLocation *ProtoLocs,
-      SourceLocation EndProtoLoc, const ParsedAttributesView &AttrList);
+      SourceLocation EndProtoLoc, const ParsedAttributesView &AttrList,
+      SkipBodyInfo *SkipBody);
 
   ObjCCategoryDecl *ActOnStartCategoryInterface(
       SourceLocation AtInterfaceLoc, IdentifierInfo *ClassName,

diff  --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp
index 64e9f3cca8777..2ba8daae11d05 100644
--- a/clang/lib/AST/DeclObjC.cpp
+++ b/clang/lib/AST/DeclObjC.cpp
@@ -1997,6 +1997,17 @@ void ObjCProtocolDecl::startDefinition() {
     RD->Data = this->Data;
 }
 
+void ObjCProtocolDecl::startDuplicateDefinitionForComparison() {
+  Data.setPointer(nullptr);
+  allocateDefinitionData();
+  // Don't propagate data to other redeclarations.
+}
+
+void ObjCProtocolDecl::mergeDuplicateDefinitionWithCommon(
+    const ObjCProtocolDecl *Definition) {
+  Data = Definition->Data;
+}
+
 void ObjCProtocolDecl::collectPropertiesToImplement(PropertyMap &PM) const {
   if (const ObjCProtocolDecl *PDecl = getDefinition()) {
     for (auto *Prop : PDecl->properties()) {

diff  --git a/clang/lib/AST/ODRDiagsEmitter.cpp b/clang/lib/AST/ODRDiagsEmitter.cpp
index dfc2157cacac9..6e0f1226de721 100644
--- a/clang/lib/AST/ODRDiagsEmitter.cpp
+++ b/clang/lib/AST/ODRDiagsEmitter.cpp
@@ -90,8 +90,9 @@ static bool diagnoseSubMismatchMethodParameters(DiagnosticsEngine &Diags,
     DiagMethodType SecondMethodType = GetDiagMethodType(SecondMethod);
     return Diags.Report(SecondMethod->getLocation(),
                         diag::note_module_odr_violation_method_params)
-           << SecondModule << SecondMethod->getSourceRange() << DiffType
-           << SecondMethodType << SecondName;
+           << SecondModule.empty() << SecondModule
+           << SecondMethod->getSourceRange() << DiffType << SecondMethodType
+           << SecondName;
   };
 
   const unsigned FirstNumParameters = FirstMethod->param_size();
@@ -378,7 +379,7 @@ bool ODRDiagsEmitter::diagnoseSubMismatchProtocols(
                               this](SourceLocation Loc, SourceRange Range,
                                     ODRReferencedProtocolDifference DiffType) {
     return Diag(Loc, diag::note_module_odr_violation_referenced_protocols)
-           << SecondModule << Range << DiffType;
+           << SecondModule.empty() << SecondModule << Range << DiffType;
   };
   auto GetProtoListSourceRange = [](const ObjCProtocolList &PL) {
     if (PL.empty())
@@ -440,7 +441,8 @@ bool ODRDiagsEmitter::diagnoseSubMismatchObjCMethod(
                    this](ODRMethodDifference DiffType) {
     return Diag(SecondMethod->getLocation(),
                 diag::note_module_odr_violation_objc_method)
-           << SecondModule << SecondMethod->getSourceRange() << DiffType;
+           << SecondModule.empty() << SecondModule
+           << SecondMethod->getSourceRange() << DiffType;
   };
 
   if (computeODRHash(FirstMethod->getReturnType()) !=
@@ -517,7 +519,8 @@ bool ODRDiagsEmitter::diagnoseSubMismatchObjCProperty(
   auto DiagNote = [SecondModule, SecondProp,
                    this](SourceLocation Loc, ODRPropertyDifference DiffType) {
     return Diag(Loc, diag::note_module_odr_violation_objc_property)
-           << SecondModule << SecondProp->getSourceRange() << DiffType;
+           << SecondModule.empty() << SecondModule
+           << SecondProp->getSourceRange() << DiffType;
   };
 
   IdentifierInfo *FirstII = FirstProp->getIdentifier();
@@ -690,7 +693,8 @@ void ODRDiagsEmitter::diagnoseSubMismatchDifferentDeclKinds(
   auto SecondDiagInfo =
       GetMismatchedDeclLoc(SecondRecord, DR.SecondDiffType, DR.SecondDecl);
   Diag(SecondDiagInfo.first, diag::note_module_odr_violation_mismatch_decl)
-      << SecondModule << SecondDiagInfo.second << DR.SecondDiffType;
+      << SecondModule.empty() << SecondModule << SecondDiagInfo.second
+      << DR.SecondDiffType;
 }
 
 bool ODRDiagsEmitter::diagnoseMismatch(
@@ -1538,7 +1542,8 @@ bool ODRDiagsEmitter::diagnoseMismatch(
       << FirstDecl->getSourceRange();
   Diag(SecondDecl->getLocation(),
        diag::note_module_odr_violation_mismatch_decl_unknown)
-      << SecondModule << FirstDiffType << SecondDecl->getSourceRange();
+      << SecondModule.empty() << SecondModule << FirstDiffType
+      << SecondDecl->getSourceRange();
   return true;
 }
 
@@ -1908,6 +1913,7 @@ bool ODRDiagsEmitter::diagnoseMismatch(
       << FirstDecl->getSourceRange();
   Diag(SecondDecl->getLocation(),
        diag::note_module_odr_violation_mismatch_decl_unknown)
-      << SecondModule << FirstDiffType << SecondDecl->getSourceRange();
+      << SecondModule.empty() << SecondModule << FirstDiffType
+      << SecondDecl->getSourceRange();
   return true;
 }

diff  --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index 734c66f65dc29..7404eba0053a6 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ODRDiagsEmitter.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/TargetInfo.h"
@@ -2088,11 +2089,23 @@ Parser::ParseObjCAtProtocolDeclaration(SourceLocation AtLoc,
                                   /*consumeLastToken=*/true))
     return nullptr;
 
-  Decl *ProtoType = Actions.ActOnStartProtocolInterface(
+  Sema::SkipBodyInfo SkipBody;
+  ObjCProtocolDecl *ProtoType = Actions.ActOnStartProtocolInterface(
       AtLoc, protocolName, nameLoc, ProtocolRefs.data(), ProtocolRefs.size(),
-      ProtocolLocs.data(), EndProtoLoc, attrs);
+      ProtocolLocs.data(), EndProtoLoc, attrs, &SkipBody);
 
   ParseObjCInterfaceDeclList(tok::objc_protocol, ProtoType);
+  if (SkipBody.CheckSameAsPrevious) {
+    auto *PreviousDef = cast<ObjCProtocolDecl>(SkipBody.Previous);
+    if (Actions.ActOnDuplicateODRHashDefinition(ProtoType, PreviousDef)) {
+      ProtoType->mergeDuplicateDefinitionWithCommon(
+          PreviousDef->getDefinition());
+    } else {
+      ODRDiagsEmitter DiagsEmitter(Diags, Actions.getASTContext(),
+                                   getPreprocessor().getLangOpts());
+      DiagsEmitter.diagnoseMismatch(PreviousDef, ProtoType);
+    }
+  }
   return Actions.ConvertDeclToDeclGroup(ProtoType);
 }
 

diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index c55cb7842072b..97dff49862bb5 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -1213,7 +1213,7 @@ ObjCProtocolDecl *Sema::ActOnStartProtocolInterface(
     SourceLocation AtProtoInterfaceLoc, IdentifierInfo *ProtocolName,
     SourceLocation ProtocolLoc, Decl *const *ProtoRefs, unsigned NumProtoRefs,
     const SourceLocation *ProtoLocs, SourceLocation EndProtoLoc,
-    const ParsedAttributesView &AttrList) {
+    const ParsedAttributesView &AttrList, SkipBodyInfo *SkipBody) {
   bool err = false;
   // FIXME: Deal with AttrList.
   assert(ProtocolName && "Missing protocol identifier");
@@ -1221,23 +1221,29 @@ ObjCProtocolDecl *Sema::ActOnStartProtocolInterface(
                                               forRedeclarationInCurContext());
   ObjCProtocolDecl *PDecl = nullptr;
   if (ObjCProtocolDecl *Def = PrevDecl? PrevDecl->getDefinition() : nullptr) {
-    // If we already have a definition, complain.
-    Diag(ProtocolLoc, diag::warn_duplicate_protocol_def) << ProtocolName;
-    Diag(Def->getLocation(), diag::note_previous_definition);
-
     // Create a new protocol that is completely distinct from previous
     // declarations, and do not make this protocol available for name lookup.
     // That way, we'll end up completely ignoring the duplicate.
     // FIXME: Can we turn this into an error?
     PDecl = ObjCProtocolDecl::Create(Context, CurContext, ProtocolName,
                                      ProtocolLoc, AtProtoInterfaceLoc,
-                                     /*PrevDecl=*/nullptr);
+                                     /*PrevDecl=*/Def);
+
+    if (SkipBody && !hasVisibleDefinition(Def)) {
+      SkipBody->CheckSameAsPrevious = true;
+      SkipBody->New = PDecl;
+      SkipBody->Previous = Def;
+    } else {
+      // If we already have a definition, complain.
+      Diag(ProtocolLoc, diag::warn_duplicate_protocol_def) << ProtocolName;
+      Diag(Def->getLocation(), diag::note_previous_definition);
+    }
 
     // If we are using modules, add the decl to the context in order to
     // serialize something meaningful.
     if (getLangOpts().Modules)
       PushOnScopeChains(PDecl, TUScope);
-    PDecl->startDefinition();
+    PDecl->startDuplicateDefinitionForComparison();
   } else {
     if (PrevDecl) {
       // Check for circular dependencies among protocol declarations. This can

diff  --git a/clang/test/Modules/compare-objc-protocol.m b/clang/test/Modules/compare-objc-protocol.m
index 86550b26965f5..6dec2965487c5 100644
--- a/clang/test/Modules/compare-objc-protocol.m
+++ b/clang/test/Modules/compare-objc-protocol.m
@@ -19,6 +19,11 @@
 // RUN: %clang_cc1 -I%t/include -verify %t/test.m -fblocks -fobjc-arc \
 // RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
 
+// Run the same test with second.h being modular
+// RUN: cat %t/include/second.modulemap >> %t/include/module.modulemap
+// RUN: %clang_cc1 -I%t/include -verify %t/test.m -fblocks -fobjc-arc -DTEST_MODULAR=1 \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
 // In non-modular case we ignore protocol redefinitions. But with modules
 // previous definition can come from a hidden [sub]module. And in this case we
 // allow a new definition if it is equivalent to the hidden one.
@@ -47,6 +52,7 @@ @protocol ExtraProtocol @end
     export *
   }
 }
+//--- include/second.modulemap
 module Second {
   header "second.h"
   export *
@@ -99,17 +105,21 @@ @protocol CompareProtocolOrder<ExtraProtocol, CommonProtocol> @end
 
 id<CompareProtocolPresence1> compareProtocolPresence1;
 // expected-error at first.h:* {{'CompareProtocolPresence1' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found 1 referenced protocol}}
+#ifdef TEST_MODULAR
 // expected-note at second.h:* {{but in 'Second' found 0 referenced protocols}}
+#else
+// expected-note at second.h:* {{but in definition here found 0 referenced protocols}}
+#endif
 id<CompareProtocolPresence2> compareProtocolPresence2;
 // expected-error at first.h:* {{'CompareProtocolPresence2' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found 0 referenced protocols}}
-// expected-note at second.h:* {{but in 'Second' found 1 referenced protocol}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found 1 referenced protocol}}
 
 id<CompareDifferentProtocols> compareDifferentProtocols;
 // expected-error at first.h:* {{'CompareDifferentProtocols' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found 1st referenced protocol with name 'CommonProtocol'}}
-// expected-note at second.h:* {{but in 'Second' found 1st referenced protocol with 
diff erent name 'ExtraProtocol'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found 1st referenced protocol with 
diff erent name 'ExtraProtocol'}}
 id<CompareProtocolOrder> compareProtocolOrder;
 // expected-error at first.h:* {{'CompareProtocolOrder' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found 1st referenced protocol with name 'CommonProtocol'}}
-// expected-note at second.h:* {{but in 'Second' found 1st referenced protocol with 
diff erent name 'ExtraProtocol'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found 1st referenced protocol with 
diff erent name 'ExtraProtocol'}}
 #endif
 
 #if defined(FIRST)
@@ -208,38 +218,38 @@ - (void)methodRequiredness;
 id<CompareMatchingMethods> compareMatchingMethods; // no error
 id<CompareMethodPresence1> compareMethodPresence1;
 // expected-error at first.h:* {{'CompareMethodPresence1' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found method}}
-// expected-note at second.h:* {{but in 'Second' found end of class}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found end of class}}
 id<CompareMethodPresence2> compareMethodPresence2;
 // expected-error at first.h:* {{'CompareMethodPresence2' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found end of class}}
-// expected-note at second.h:* {{but in 'Second' found method}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found method}}
 id<CompareMethodName> compareMethodName;
 // expected-error at first.h:* {{'CompareMethodName' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found method 'methodNameA'}}
-// expected-note at second.h:* {{but in 'Second' found 
diff erent method 'methodNameB'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found 
diff erent method 'methodNameB'}}
 
 id<CompareMethodArgCount> compareMethodArgCount;
 // expected-error at first.h:* {{'CompareMethodArgCount' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found method 'methodArgCount::' that has 2 parameters}}
-// expected-note at second.h:* {{but in 'Second' found method 'methodArgCount:' that has 1 parameter}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found method 'methodArgCount:' that has 1 parameter}}
 id<CompareMethodArgName> compareMethodArgName;
 // expected-error at first.h:* {{'CompareMethodArgName' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found method 'methodArgName:' with 1st parameter named 'argNameA'}}
-// expected-note at second.h:* {{but in 'Second' found method 'methodArgName:' with 1st parameter named 'argNameB'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found method 'methodArgName:' with 1st parameter named 'argNameB'}}
 id<CompareMethodArgType> compareMethodArgType;
 // expected-error at first.h:* {{'CompareMethodArgType' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found method 'methodArgType:' with 1st parameter of type 'int'}}
-// expected-note at second.h:* {{but in 'Second' found method 'methodArgType:' with 1st parameter of type 'float'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found method 'methodArgType:' with 1st parameter of type 'float'}}
 
 id<CompareMethodReturnType> compareMethodReturnType;
 // expected-error at first.h:* {{'CompareMethodReturnType' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found method 'methodReturnType' with return type 'int'}}
-// expected-note at second.h:* {{but in 'Second' found method 'methodReturnType' with 
diff erent return type 'float'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found method 'methodReturnType' with 
diff erent return type 'float'}}
 
 id<CompareMethodOrder> compareMethodOrder;
 // expected-error at first.h:* {{'CompareMethodOrder' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found method 'methodOrderFirst'}}
-// expected-note at second.h:* {{but in 'Second' found 
diff erent method 'methodOrderSecond'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found 
diff erent method 'methodOrderSecond'}}
 id<CompareMethodClassInstance> compareMethodClassInstance;
 // expected-error at first.h:* {{'CompareMethodClassInstance' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found instance method 'methodClassInstance'}}
-// expected-note at second.h:* {{but in 'Second' found method 'methodClassInstance' as class method}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found method 'methodClassInstance' as class method}}
 
 id<CompareMethodRequirednessExplicit> compareMethodRequirednessExplicit;
 // expected-error at first.h:* {{'CompareMethodRequirednessExplicit' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found 'optional' method control}}
-// expected-note at second.h:* {{but in 'Second' found 'required' method control}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found 'required' method control}}
 id<CompareMethodRequirednessDefault> compareMethodRequirednessDefault; // no error
 #endif
 
@@ -322,28 +332,28 @@ @protocol CompareLastImplAttribute
 id<CompareMatchingProperties> compareMatchingProperties;
 id<ComparePropertyPresence1> comparePropertyPresence1;
 // expected-error at first.h:* {{'ComparePropertyPresence1' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property}}
-// expected-note at second.h:* {{but in 'Second' found end of class}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found end of class}}
 id<ComparePropertyPresence2> comparePropertyPresence2;
 // expected-error at first.h:* {{'ComparePropertyPresence2' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found end of class}}
-// expected-note at second.h:* {{but in 'Second' found property}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found property}}
 id<ComparePropertyName> comparePropertyName;
 // expected-error at first.h:* {{'ComparePropertyName' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property 'propNameA'}}
-// expected-note at second.h:* {{but in 'Second' found property 'propNameB'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found property 'propNameB'}}
 id<ComparePropertyType> comparePropertyType;
 // expected-error at first.h:* {{'ComparePropertyType' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property 'propType' with type 'int'}}
-// expected-note at second.h:* {{but in 'Second' found property 'propType' with type 'float'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found property 'propType' with type 'float'}}
 id<ComparePropertyOrder> comparePropertyOrder;
 // expected-error at first.h:* {{'ComparePropertyOrder' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property 'propOrderX'}}
-// expected-note at second.h:* {{but in 'Second' found property 'propOrderY'}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found property 'propOrderY'}}
 
 id<CompareMatchingPropertyAttributes> compareMatchingPropertyAttributes;
 id<ComparePropertyAttributes> comparePropertyAttributes;
 // expected-error at first.h:* {{'ComparePropertyAttributes' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property 'propAttributes' with 'nonatomic' attribute}}
-// expected-note at second.h:* {{but in 'Second' found property 'propAttributes' with 
diff erent 'nonatomic' attribute}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found property 'propAttributes' with 
diff erent 'nonatomic' attribute}}
 id<CompareFirstImplAttribute> compareFirstImplAttribute;
 // expected-error at first.h:* {{'CompareFirstImplAttribute' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property 'firstImplAttribute' with default 'readonly' attribute}}
-// expected-note at second.h:* {{but in 'Second' found property 'firstImplAttribute' with 
diff erent 'readonly' attribute}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found property 'firstImplAttribute' with 
diff erent 'readonly' attribute}}
 id<CompareLastImplAttribute> compareLastImplAttribute;
 // expected-error at first.h:* {{'CompareLastImplAttribute' has 
diff erent definitions in 
diff erent modules; first 
diff erence is definition in module 'First.Hidden' found property 'lastImplAttribute' with 'class' attribute}}
-// expected-note at second.h:* {{but in 'Second' found property 'lastImplAttribute' with 
diff erent 'class' attribute}}
+// expected-note-re at second.h:* {{but in {{'Second'|definition here}} found property 'lastImplAttribute' with 
diff erent 'class' attribute}}
 #endif

diff  --git a/clang/test/Modules/hidden-duplicates.m b/clang/test/Modules/hidden-duplicates.m
new file mode 100644
index 0000000000000..de46a4ab56c2f
--- /dev/null
+++ b/clang/test/Modules/hidden-duplicates.m
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -DTEST_MAKE_HIDDEN_VISIBLE=1 \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -x objective-c++ \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify -DTEST_MAKE_HIDDEN_VISIBLE=1 -x objective-c++ \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Test parsing duplicate Objective-C entities when a previous entity is defined
+// in a hidden [sub]module and cannot be used.
+//
+// Testing with header guards and includes on purpose as tracking imports in
+// modules is a separate issue.
+
+//--- include/textual.h
+#ifndef TEXTUAL_H
+#define TEXTUAL_H
+
+ at protocol TestProtocol
+- (void)someMethod;
+ at end
+
+ at protocol ForwardDeclaredProtocolWithoutDefinition;
+
+id<TestProtocol> protocolDefinition(id<TestProtocol> t);
+id<ForwardDeclaredProtocolWithoutDefinition> forwardDeclaredProtocol(
+    id<ForwardDeclaredProtocolWithoutDefinition> t);
+
+#endif
+
+//--- include/empty.h
+//--- include/initially_hidden.h
+#include "textual.h"
+
+//--- include/module.modulemap
+module Piecewise {
+  module Empty {
+    header "empty.h"
+  }
+  module InitiallyHidden {
+    header "initially_hidden.h"
+    export *
+  }
+}
+
+//--- test.m
+// Including empty.h loads the entire module Piecewise but keeps InitiallyHidden hidden.
+#include "empty.h"
+#include "textual.h"
+#ifdef TEST_MAKE_HIDDEN_VISIBLE
+#include "initially_hidden.h"
+#endif
+// expected-no-diagnostics


        


More information about the cfe-commits mailing list