diff --git a/clang/include/clang/AST/ODRDiagsEmitter.h b/clang/include/clang/AST/ODRDiagsEmitter.h --- a/clang/include/clang/AST/ODRDiagsEmitter.h +++ b/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; diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/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< diff --git a/clang/lib/AST/ODRDiagsEmitter.cpp b/clang/lib/AST/ODRDiagsEmitter.cpp --- a/clang/lib/AST/ODRDiagsEmitter.cpp +++ b/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(FirstDecl), + cast(SecondDecl))) + return true; + break; + } } Diag(FirstDecl->getLocation(), diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp --- a/clang/lib/AST/ODRHash.cpp +++ b/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; } } diff --git a/clang/test/Modules/compare-objc-protocol.m b/clang/test/Modules/compare-objc-protocol.m --- a/clang/test/Modules/compare-objc-protocol.m +++ b/clang/test/Modules/compare-objc-protocol.m @@ -242,3 +242,108 @@ // expected-note@second.h:* {{but in 'Second' found 'required' method control}} id 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; +id 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; +// 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; +// 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; +// 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; +// 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; +id 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; +// 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; +// 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