[clang] dcb71b5 - [ODRHash] Hash `ObjCPropertyDecl` and diagnose discovered mismatches.

Volodymyr Sapsai via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 17 17:22:36 PST 2022


Author: Volodymyr Sapsai
Date: 2022-11-17T17:22:03-08:00
New Revision: dcb71b5e1d1311c41f409c8dab26e04b084875be

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

LOG: [ODRHash] Hash `ObjCPropertyDecl` and diagnose discovered mismatches.

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

Added: 
    

Modified: 
    clang/include/clang/AST/ODRDiagsEmitter.h
    clang/include/clang/Basic/DiagnosticASTKinds.td
    clang/lib/AST/ODRDiagsEmitter.cpp
    clang/lib/AST/ODRHash.cpp
    clang/test/Modules/compare-objc-protocol.m

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ODRDiagsEmitter.h b/clang/include/clang/AST/ODRDiagsEmitter.h
index cbabaa7e69b41..4c41a4a6004f1 100644
--- a/clang/include/clang/AST/ODRDiagsEmitter.h
+++ b/clang/include/clang/AST/ODRDiagsEmitter.h
@@ -81,6 +81,7 @@ class ODRDiagsEmitter {
     Friend,
     FunctionTemplate,
     ObjCMethod,
+    ObjCProperty,
     Other
   };
 
@@ -149,6 +150,15 @@ class ODRDiagsEmitter {
                                      const ObjCMethodDecl *FirstMethod,
                                      const ObjCMethodDecl *SecondMethod) const;
 
+  /// Check if Objective-C properties are the same and diagnose if 
diff erent.
+  ///
+  /// Returns true if found a mismatch and diagnosed it.
+  bool
+  diagnoseSubMismatchObjCProperty(const NamedDecl *FirstObjCContainer,
+                                  StringRef FirstModule, StringRef SecondModule,
+                                  const ObjCPropertyDecl *FirstProp,
+                                  const ObjCPropertyDecl *SecondProp) const;
+
 private:
   DiagnosticsEngine &Diags;
   const ASTContext &Context;

diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index af7aec65bf6c8..0f7e256d8056f 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -621,11 +621,11 @@ def err_module_odr_violation_mismatch_decl : Error<
   "%select{definition in module '%2'|defined here}1 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}3">;
+  "data member|friend declaration|function template|method|property}3">;
 def note_module_odr_violation_mismatch_decl : Note<"but in '%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}1">;
+  "data member|friend declaration|function template|method|property}1">;
 
 def err_module_odr_violation_record : Error<
   "%q0 has 
diff erent definitions in 
diff erent modules; first 
diff erence is "
@@ -891,18 +891,40 @@ def note_module_odr_violation_method_params : Note<"but in '%0' found "
     "with %ordinal4 parameter named %5"
   "}1">;
 
+def err_module_odr_violation_objc_property : Error<
+  "%q0 has 
diff erent definitions in 
diff erent modules; first 
diff erence is "
+  "%select{definition in module '%2'|defined here}1 found "
+  "%select{"
+  "property %4|"
+  "property %4 with type %5|"
+  "%select{no|'required'|'optional'}4 property control|"
+  "property %4 with %select{default |}6'%select{none|readonly|getter|assign|"
+    "readwrite|retain|copy|nonatomic|setter|atomic|weak|strong|"
+    "unsafe_unretained|nullability|null_resettable|class|direct}5' attribute"
+  "}3">;
+def note_module_odr_violation_objc_property : Note<
+  "but in '%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|"
+    "readwrite|retain|copy|nonatomic|setter|atomic|weak|strong|"
+    "unsafe_unretained|nullability|null_resettable|class|direct}3' attribute"
+  "}1">;
+
 def err_module_odr_violation_mismatch_decl_unknown : Error<
   "%q0 %select{with definition in module '%2'|defined here}1 has 
diff erent "
   "definitions in 
diff erent modules; first 
diff erence is this "
   "%select{||||static assert|field|method|type alias|typedef|data member|"
   "friend declaration|function template|method|"
-  "unexpected decl}3">;
+  "property|unexpected decl}3">;
 def note_module_odr_violation_mismatch_decl_unknown : Note<
   "but in '%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|"
-  "another unexpected decl}1">;
+  "
diff erent property|another unexpected decl}1">;
 
 
 def remark_sanitize_address_insert_extra_padding_accepted : Remark<

diff  --git a/clang/lib/AST/ODRDiagsEmitter.cpp b/clang/lib/AST/ODRDiagsEmitter.cpp
index 429bb16a17c5f..dfc2157cacac9 100644
--- a/clang/lib/AST/ODRDiagsEmitter.cpp
+++ b/clang/lib/AST/ODRDiagsEmitter.cpp
@@ -497,6 +497,81 @@ bool ODRDiagsEmitter::diagnoseSubMismatchObjCMethod(
   return false;
 }
 
+bool ODRDiagsEmitter::diagnoseSubMismatchObjCProperty(
+    const NamedDecl *FirstObjCContainer, StringRef FirstModule,
+    StringRef SecondModule, const ObjCPropertyDecl *FirstProp,
+    const ObjCPropertyDecl *SecondProp) const {
+  enum ODRPropertyDifference {
+    Name,
+    Type,
+    ControlLevel, // optional/required
+    Attribute,
+  };
+
+  auto DiagError = [FirstObjCContainer, FirstModule, FirstProp,
+                    this](SourceLocation Loc, ODRPropertyDifference DiffType) {
+    return Diag(Loc, diag::err_module_odr_violation_objc_property)
+           << FirstObjCContainer << FirstModule.empty() << FirstModule
+           << FirstProp->getSourceRange() << DiffType;
+  };
+  auto DiagNote = [SecondModule, SecondProp,
+                   this](SourceLocation Loc, ODRPropertyDifference DiffType) {
+    return Diag(Loc, diag::note_module_odr_violation_objc_property)
+           << SecondModule << SecondProp->getSourceRange() << DiffType;
+  };
+
+  IdentifierInfo *FirstII = FirstProp->getIdentifier();
+  IdentifierInfo *SecondII = SecondProp->getIdentifier();
+  if (FirstII->getName() != SecondII->getName()) {
+    DiagError(FirstProp->getLocation(), Name) << FirstII;
+    DiagNote(SecondProp->getLocation(), Name) << SecondII;
+    return true;
+  }
+  if (computeODRHash(FirstProp->getType()) !=
+      computeODRHash(SecondProp->getType())) {
+    DiagError(FirstProp->getLocation(), Type)
+        << FirstII << FirstProp->getType();
+    DiagNote(SecondProp->getLocation(), Type)
+        << SecondII << SecondProp->getType();
+    return true;
+  }
+  if (FirstProp->getPropertyImplementation() !=
+      SecondProp->getPropertyImplementation()) {
+    DiagError(FirstProp->getLocation(), ControlLevel)
+        << FirstProp->getPropertyImplementation();
+    DiagNote(SecondProp->getLocation(), ControlLevel)
+        << SecondProp->getPropertyImplementation();
+    return true;
+  }
+
+  // Go over the property attributes and stop at the first mismatch.
+  unsigned FirstAttrs = (unsigned)FirstProp->getPropertyAttributes();
+  unsigned SecondAttrs = (unsigned)SecondProp->getPropertyAttributes();
+  if (FirstAttrs != SecondAttrs) {
+    for (unsigned I = 0; I < NumObjCPropertyAttrsBits; ++I) {
+      unsigned CheckedAttr = (1 << I);
+      if ((FirstAttrs & CheckedAttr) == (SecondAttrs & CheckedAttr))
+        continue;
+
+      bool IsFirstWritten =
+          (unsigned)FirstProp->getPropertyAttributesAsWritten() & CheckedAttr;
+      bool IsSecondWritten =
+          (unsigned)SecondProp->getPropertyAttributesAsWritten() & CheckedAttr;
+      DiagError(IsFirstWritten ? FirstProp->getLParenLoc()
+                               : FirstProp->getLocation(),
+                Attribute)
+          << FirstII << (I + 1) << IsFirstWritten;
+      DiagNote(IsSecondWritten ? SecondProp->getLParenLoc()
+                               : SecondProp->getLocation(),
+               Attribute)
+          << SecondII << (I + 1);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 ODRDiagsEmitter::DiffResult
 ODRDiagsEmitter::FindTypeDiffs(DeclHashes &FirstHashes,
                                DeclHashes &SecondHashes) {
@@ -537,6 +612,8 @@ ODRDiagsEmitter::FindTypeDiffs(DeclHashes &FirstHashes,
       return FunctionTemplate;
     case Decl::ObjCMethod:
       return ObjCMethod;
+    case Decl::ObjCProperty:
+      return ObjCProperty;
     }
   };
 
@@ -889,6 +966,7 @@ bool ODRDiagsEmitter::diagnoseMismatch(
   case PrivateSpecifer:
   case ProtectedSpecifer:
   case ObjCMethod:
+  case ObjCProperty:
     llvm_unreachable("Invalid 
diff  type");
 
   case StaticAssert: {
@@ -1814,6 +1892,14 @@ bool ODRDiagsEmitter::diagnoseMismatch(
       return true;
     break;
   }
+  case ObjCProperty: {
+    if (diagnoseSubMismatchObjCProperty(FirstProtocol, FirstModule,
+                                        SecondModule,
+                                        cast<ObjCPropertyDecl>(FirstDecl),
+                                        cast<ObjCPropertyDecl>(SecondDecl)))
+      return true;
+    break;
+  }
   }
 
   Diag(FirstDecl->getLocation(),

diff  --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp
index 384259c72d810..fee47881a1835 100644
--- a/clang/lib/AST/ODRHash.cpp
+++ b/clang/lib/AST/ODRHash.cpp
@@ -337,6 +337,15 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
     Inherited::VisitFieldDecl(D);
   }
 
+  void VisitObjCPropertyDecl(const ObjCPropertyDecl *D) {
+    ID.AddInteger(D->getPropertyAttributes());
+    ID.AddInteger(D->getPropertyImplementation());
+    AddQualType(D->getType());
+    AddDecl(D);
+
+    Inherited::VisitObjCPropertyDecl(D);
+  }
+
   void VisitFunctionDecl(const FunctionDecl *D) {
     // Handled by the ODRHash for FunctionDecl
     ID.AddInteger(D->getODRHash());
@@ -522,6 +531,7 @@ bool ODRHash::isSubDeclToBeProcessed(const Decl *D, const DeclContext *Parent) {
     case Decl::Typedef:
     case Decl::Var:
     case Decl::ObjCMethod:
+    case Decl::ObjCProperty:
       return true;
   }
 }

diff  --git a/clang/test/Modules/compare-objc-protocol.m b/clang/test/Modules/compare-objc-protocol.m
index 706a4b2e85341..86550b26965f5 100644
--- a/clang/test/Modules/compare-objc-protocol.m
+++ b/clang/test/Modules/compare-objc-protocol.m
@@ -242,3 +242,108 @@ - (void)methodRequiredness;
 // expected-note at second.h:* {{but in 'Second' found 'required' method control}}
 id<CompareMethodRequirednessDefault> compareMethodRequirednessDefault; // no error
 #endif
+
+#if defined(FIRST)
+ at protocol CompareMatchingProperties
+ at property int matchingPropName;
+ at end
+
+ at protocol ComparePropertyPresence1
+ at property int propPresence1;
+ at end
+ at protocol ComparePropertyPresence2
+ at end
+
+ at protocol ComparePropertyName
+ at property int propNameA;
+ at end
+
+ at protocol ComparePropertyType
+ at property int propType;
+ at end
+
+ at protocol ComparePropertyOrder
+ at property int propOrderX;
+ at property int propOrderY;
+ at end
+
+ at protocol CompareMatchingPropertyAttributes
+ at property (nonatomic, assign) int matchingProp;
+ at end
+ at protocol ComparePropertyAttributes
+ at property (nonatomic) int propAttributes;
+ at end
+// Edge cases.
+ at protocol CompareFirstImplAttribute
+ at property int firstImplAttribute;
+ at end
+ at protocol CompareLastImplAttribute
+// Cannot test with protocols 'direct' attribute because it's not allowed.
+ at property (class) int lastImplAttribute;
+ at end
+#elif defined(SECOND)
+ at protocol CompareMatchingProperties
+ at property int matchingPropName;
+ at end
+
+ at protocol ComparePropertyPresence1
+ at end
+ at protocol ComparePropertyPresence2
+ at property int propPresence2;
+ at end
+
+ at protocol ComparePropertyName
+ at property int propNameB;
+ at end
+
+ at protocol ComparePropertyType
+ at property float propType;
+ at end
+
+ at protocol ComparePropertyOrder
+ at property int propOrderY;
+ at property int propOrderX;
+ at end
+
+ at protocol CompareMatchingPropertyAttributes
+ at property (assign, nonatomic) int matchingProp;
+ at end
+ at protocol ComparePropertyAttributes
+ at property (atomic) int propAttributes;
+ at end
+// Edge cases.
+ at protocol CompareFirstImplAttribute
+ at property (readonly) int firstImplAttribute;
+ at end
+ at protocol CompareLastImplAttribute
+ at property int lastImplAttribute;
+ at end
+#else
+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}}
+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}}
+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'}}
+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'}}
+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'}}
+
+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}}
+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}}
+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}}
+#endif


        


More information about the cfe-commits mailing list