This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdcb71b5e1d13: [ODRHash] Hash `ObjCPropertyDecl` and diagnose
discovered mismatches. (authored by vsapsai).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130326/new/
https://reviews.llvm.org/D130326
Files:
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
Index: clang/test/Modules/compare-objc-protocol.m
===================================================================
--- clang/test/Modules/compare-objc-protocol.m
+++ clang/test/Modules/compare-objc-protocol.m
@@ -242,3 +242,108 @@
// expected-note@second.h:* {{but in 'Second' found 'required' method control}}
id<CompareMethodRequirednessDefault> compareMethodRequirednessDefault; // no error
#endif
+
+#if defined(FIRST)
+@protocol CompareMatchingProperties
+@property int matchingPropName;
+@end
+
+@protocol ComparePropertyPresence1
+@property int propPresence1;
+@end
+@protocol ComparePropertyPresence2
+@end
+
+@protocol ComparePropertyName
+@property int propNameA;
+@end
+
+@protocol ComparePropertyType
+@property int propType;
+@end
+
+@protocol ComparePropertyOrder
+@property int propOrderX;
+@property int propOrderY;
+@end
+
+@protocol CompareMatchingPropertyAttributes
+@property (nonatomic, assign) int matchingProp;
+@end
+@protocol ComparePropertyAttributes
+@property (nonatomic) int propAttributes;
+@end
+// Edge cases.
+@protocol CompareFirstImplAttribute
+@property int firstImplAttribute;
+@end
+@protocol CompareLastImplAttribute
+// Cannot test with protocols 'direct' attribute because it's not allowed.
+@property (class) int lastImplAttribute;
+@end
+#elif defined(SECOND)
+@protocol CompareMatchingProperties
+@property int matchingPropName;
+@end
+
+@protocol ComparePropertyPresence1
+@end
+@protocol ComparePropertyPresence2
+@property int propPresence2;
+@end
+
+@protocol ComparePropertyName
+@property int propNameB;
+@end
+
+@protocol ComparePropertyType
+@property float propType;
+@end
+
+@protocol ComparePropertyOrder
+@property int propOrderY;
+@property int propOrderX;
+@end
+
+@protocol CompareMatchingPropertyAttributes
+@property (assign, nonatomic) int matchingProp;
+@end
+@protocol ComparePropertyAttributes
+@property (atomic) int propAttributes;
+@end
+// Edge cases.
+@protocol CompareFirstImplAttribute
+@property (readonly) int firstImplAttribute;
+@end
+@protocol CompareLastImplAttribute
+@property int lastImplAttribute;
+@end
+#else
+id<CompareMatchingProperties> compareMatchingProperties;
+id<ComparePropertyPresence1> comparePropertyPresence1;
+// expected-error@first.h:* {{'ComparePropertyPresence1' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property}}
+// expected-note@second.h:* {{but in 'Second' found end of class}}
+id<ComparePropertyPresence2> comparePropertyPresence2;
+// expected-error@first.h:* {{'ComparePropertyPresence2' has different definitions in different modules; first difference is definition in module 'First.Hidden' found end of class}}
+// expected-note@second.h:* {{but in 'Second' found property}}
+id<ComparePropertyName> comparePropertyName;
+// expected-error@first.h:* {{'ComparePropertyName' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'propNameA'}}
+// expected-note@second.h:* {{but in 'Second' found property 'propNameB'}}
+id<ComparePropertyType> comparePropertyType;
+// expected-error@first.h:* {{'ComparePropertyType' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'propType' with type 'int'}}
+// expected-note@second.h:* {{but in 'Second' found property 'propType' with type 'float'}}
+id<ComparePropertyOrder> comparePropertyOrder;
+// expected-error@first.h:* {{'ComparePropertyOrder' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'propOrderX'}}
+// expected-note@second.h:* {{but in 'Second' found property 'propOrderY'}}
+
+id<CompareMatchingPropertyAttributes> compareMatchingPropertyAttributes;
+id<ComparePropertyAttributes> comparePropertyAttributes;
+// expected-error@first.h:* {{'ComparePropertyAttributes' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'propAttributes' with 'nonatomic' attribute}}
+// expected-note@second.h:* {{but in 'Second' found property 'propAttributes' with different 'nonatomic' attribute}}
+id<CompareFirstImplAttribute> compareFirstImplAttribute;
+// expected-error@first.h:* {{'CompareFirstImplAttribute' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'firstImplAttribute' with default 'readonly' attribute}}
+// expected-note@second.h:* {{but in 'Second' found property 'firstImplAttribute' with different 'readonly' attribute}}
+id<CompareLastImplAttribute> compareLastImplAttribute;
+// expected-error@first.h:* {{'CompareLastImplAttribute' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'lastImplAttribute' with 'class' attribute}}
+// expected-note@second.h:* {{but in 'Second' found property 'lastImplAttribute' with different 'class' attribute}}
+#endif
Index: clang/lib/AST/ODRHash.cpp
===================================================================
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -337,6 +337,15 @@
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 @@
case Decl::Typedef:
case Decl::Var:
case Decl::ObjCMethod:
+ case Decl::ObjCProperty:
return true;
}
}
Index: clang/lib/AST/ODRDiagsEmitter.cpp
===================================================================
--- clang/lib/AST/ODRDiagsEmitter.cpp
+++ clang/lib/AST/ODRDiagsEmitter.cpp
@@ -497,6 +497,81 @@
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 @@
return FunctionTemplate;
case Decl::ObjCMethod:
return ObjCMethod;
+ case Decl::ObjCProperty:
+ return ObjCProperty;
}
};
@@ -889,6 +966,7 @@
case PrivateSpecifer:
case ProtectedSpecifer:
case ObjCMethod:
+ case ObjCProperty:
llvm_unreachable("Invalid diff type");
case StaticAssert: {
@@ -1814,6 +1892,14 @@
return true;
break;
}
+ case ObjCProperty: {
+ if (diagnoseSubMismatchObjCProperty(FirstProtocol, FirstModule,
+ SecondModule,
+ cast<ObjCPropertyDecl>(FirstDecl),
+ cast<ObjCPropertyDecl>(SecondDecl)))
+ return true;
+ break;
+ }
}
Diag(FirstDecl->getLocation(),
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -621,11 +621,11 @@
"%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 different definitions in different modules; first difference is "
@@ -891,18 +891,40 @@
"with %ordinal4 parameter named %5"
"}1">;
+def err_module_odr_violation_objc_property : Error<
+ "%q0 has different definitions in different modules; first difference 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 different '%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 different "
"definitions in different modules; first difference 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{||||different static assert|different field|different method|"
"different type alias|different typedef|different data member|"
"different friend declaration|different function template|different method|"
- "another unexpected decl}1">;
+ "different property|another unexpected decl}1">;
def remark_sanitize_address_insert_extra_padding_accepted : Remark<
Index: clang/include/clang/AST/ODRDiagsEmitter.h
===================================================================
--- clang/include/clang/AST/ODRDiagsEmitter.h
+++ clang/include/clang/AST/ODRDiagsEmitter.h
@@ -81,6 +81,7 @@
Friend,
FunctionTemplate,
ObjCMethod,
+ ObjCProperty,
Other
};
@@ -149,6 +150,15 @@
const ObjCMethodDecl *FirstMethod,
const ObjCMethodDecl *SecondMethod) const;
+ /// Check if Objective-C properties are the same and diagnose if different.
+ ///
+ /// 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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
