Skip to content

Commit dcb71b5

Browse files
committed
[ODRHash] Hash ObjCPropertyDecl and diagnose discovered mismatches.
Differential Revision: https://reviews.llvm.org/D130326
1 parent 847b5f8 commit dcb71b5

File tree

5 files changed

+237
-4
lines changed

5 files changed

+237
-4
lines changed

clang/include/clang/AST/ODRDiagsEmitter.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ class ODRDiagsEmitter {
8181
Friend,
8282
FunctionTemplate,
8383
ObjCMethod,
84+
ObjCProperty,
8485
Other
8586
};
8687

@@ -149,6 +150,15 @@ class ODRDiagsEmitter {
149150
const ObjCMethodDecl *FirstMethod,
150151
const ObjCMethodDecl *SecondMethod) const;
151152

153+
/// Check if Objective-C properties are the same and diagnose if different.
154+
///
155+
/// Returns true if found a mismatch and diagnosed it.
156+
bool
157+
diagnoseSubMismatchObjCProperty(const NamedDecl *FirstObjCContainer,
158+
StringRef FirstModule, StringRef SecondModule,
159+
const ObjCPropertyDecl *FirstProp,
160+
const ObjCPropertyDecl *SecondProp) const;
161+
152162
private:
153163
DiagnosticsEngine &Diags;
154164
const ASTContext &Context;

clang/include/clang/Basic/DiagnosticASTKinds.td

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -621,11 +621,11 @@ def err_module_odr_violation_mismatch_decl : Error<
621621
"%select{definition in module '%2'|defined here}1 found "
622622
"%select{end of class|public access specifier|private access specifier|"
623623
"protected access specifier|static assert|field|method|type alias|typedef|"
624-
"data member|friend declaration|function template|method}3">;
624+
"data member|friend declaration|function template|method|property}3">;
625625
def note_module_odr_violation_mismatch_decl : Note<"but in '%0' found "
626626
"%select{end of class|public access specifier|private access specifier|"
627627
"protected access specifier|static assert|field|method|type alias|typedef|"
628-
"data member|friend declaration|function template|method}1">;
628+
"data member|friend declaration|function template|method|property}1">;
629629

630630
def err_module_odr_violation_record : Error<
631631
"%q0 has different definitions in different modules; first difference is "
@@ -891,18 +891,40 @@ def note_module_odr_violation_method_params : Note<"but in '%0' found "
891891
"with %ordinal4 parameter named %5"
892892
"}1">;
893893

894+
def err_module_odr_violation_objc_property : Error<
895+
"%q0 has different definitions in different modules; first difference is "
896+
"%select{definition in module '%2'|defined here}1 found "
897+
"%select{"
898+
"property %4|"
899+
"property %4 with type %5|"
900+
"%select{no|'required'|'optional'}4 property control|"
901+
"property %4 with %select{default |}6'%select{none|readonly|getter|assign|"
902+
"readwrite|retain|copy|nonatomic|setter|atomic|weak|strong|"
903+
"unsafe_unretained|nullability|null_resettable|class|direct}5' attribute"
904+
"}3">;
905+
def note_module_odr_violation_objc_property : Note<
906+
"but in '%0' found "
907+
"%select{"
908+
"property %2|"
909+
"property %2 with type %3|"
910+
"%select{no|'required'|'optional'}2 property control|"
911+
"property %2 with different '%select{none|readonly|getter|assign|"
912+
"readwrite|retain|copy|nonatomic|setter|atomic|weak|strong|"
913+
"unsafe_unretained|nullability|null_resettable|class|direct}3' attribute"
914+
"}1">;
915+
894916
def err_module_odr_violation_mismatch_decl_unknown : Error<
895917
"%q0 %select{with definition in module '%2'|defined here}1 has different "
896918
"definitions in different modules; first difference is this "
897919
"%select{||||static assert|field|method|type alias|typedef|data member|"
898920
"friend declaration|function template|method|"
899-
"unexpected decl}3">;
921+
"property|unexpected decl}3">;
900922
def note_module_odr_violation_mismatch_decl_unknown : Note<
901923
"but in '%0' found "
902924
"%select{||||different static assert|different field|different method|"
903925
"different type alias|different typedef|different data member|"
904926
"different friend declaration|different function template|different method|"
905-
"another unexpected decl}1">;
927+
"different property|another unexpected decl}1">;
906928

907929

908930
def remark_sanitize_address_insert_extra_padding_accepted : Remark<

clang/lib/AST/ODRDiagsEmitter.cpp

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,81 @@ bool ODRDiagsEmitter::diagnoseSubMismatchObjCMethod(
497497
return false;
498498
}
499499

500+
bool ODRDiagsEmitter::diagnoseSubMismatchObjCProperty(
501+
const NamedDecl *FirstObjCContainer, StringRef FirstModule,
502+
StringRef SecondModule, const ObjCPropertyDecl *FirstProp,
503+
const ObjCPropertyDecl *SecondProp) const {
504+
enum ODRPropertyDifference {
505+
Name,
506+
Type,
507+
ControlLevel, // optional/required
508+
Attribute,
509+
};
510+
511+
auto DiagError = [FirstObjCContainer, FirstModule, FirstProp,
512+
this](SourceLocation Loc, ODRPropertyDifference DiffType) {
513+
return Diag(Loc, diag::err_module_odr_violation_objc_property)
514+
<< FirstObjCContainer << FirstModule.empty() << FirstModule
515+
<< FirstProp->getSourceRange() << DiffType;
516+
};
517+
auto DiagNote = [SecondModule, SecondProp,
518+
this](SourceLocation Loc, ODRPropertyDifference DiffType) {
519+
return Diag(Loc, diag::note_module_odr_violation_objc_property)
520+
<< SecondModule << SecondProp->getSourceRange() << DiffType;
521+
};
522+
523+
IdentifierInfo *FirstII = FirstProp->getIdentifier();
524+
IdentifierInfo *SecondII = SecondProp->getIdentifier();
525+
if (FirstII->getName() != SecondII->getName()) {
526+
DiagError(FirstProp->getLocation(), Name) << FirstII;
527+
DiagNote(SecondProp->getLocation(), Name) << SecondII;
528+
return true;
529+
}
530+
if (computeODRHash(FirstProp->getType()) !=
531+
computeODRHash(SecondProp->getType())) {
532+
DiagError(FirstProp->getLocation(), Type)
533+
<< FirstII << FirstProp->getType();
534+
DiagNote(SecondProp->getLocation(), Type)
535+
<< SecondII << SecondProp->getType();
536+
return true;
537+
}
538+
if (FirstProp->getPropertyImplementation() !=
539+
SecondProp->getPropertyImplementation()) {
540+
DiagError(FirstProp->getLocation(), ControlLevel)
541+
<< FirstProp->getPropertyImplementation();
542+
DiagNote(SecondProp->getLocation(), ControlLevel)
543+
<< SecondProp->getPropertyImplementation();
544+
return true;
545+
}
546+
547+
// Go over the property attributes and stop at the first mismatch.
548+
unsigned FirstAttrs = (unsigned)FirstProp->getPropertyAttributes();
549+
unsigned SecondAttrs = (unsigned)SecondProp->getPropertyAttributes();
550+
if (FirstAttrs != SecondAttrs) {
551+
for (unsigned I = 0; I < NumObjCPropertyAttrsBits; ++I) {
552+
unsigned CheckedAttr = (1 << I);
553+
if ((FirstAttrs & CheckedAttr) == (SecondAttrs & CheckedAttr))
554+
continue;
555+
556+
bool IsFirstWritten =
557+
(unsigned)FirstProp->getPropertyAttributesAsWritten() & CheckedAttr;
558+
bool IsSecondWritten =
559+
(unsigned)SecondProp->getPropertyAttributesAsWritten() & CheckedAttr;
560+
DiagError(IsFirstWritten ? FirstProp->getLParenLoc()
561+
: FirstProp->getLocation(),
562+
Attribute)
563+
<< FirstII << (I + 1) << IsFirstWritten;
564+
DiagNote(IsSecondWritten ? SecondProp->getLParenLoc()
565+
: SecondProp->getLocation(),
566+
Attribute)
567+
<< SecondII << (I + 1);
568+
return true;
569+
}
570+
}
571+
572+
return false;
573+
}
574+
500575
ODRDiagsEmitter::DiffResult
501576
ODRDiagsEmitter::FindTypeDiffs(DeclHashes &FirstHashes,
502577
DeclHashes &SecondHashes) {
@@ -537,6 +612,8 @@ ODRDiagsEmitter::FindTypeDiffs(DeclHashes &FirstHashes,
537612
return FunctionTemplate;
538613
case Decl::ObjCMethod:
539614
return ObjCMethod;
615+
case Decl::ObjCProperty:
616+
return ObjCProperty;
540617
}
541618
};
542619

@@ -889,6 +966,7 @@ bool ODRDiagsEmitter::diagnoseMismatch(
889966
case PrivateSpecifer:
890967
case ProtectedSpecifer:
891968
case ObjCMethod:
969+
case ObjCProperty:
892970
llvm_unreachable("Invalid diff type");
893971

894972
case StaticAssert: {
@@ -1814,6 +1892,14 @@ bool ODRDiagsEmitter::diagnoseMismatch(
18141892
return true;
18151893
break;
18161894
}
1895+
case ObjCProperty: {
1896+
if (diagnoseSubMismatchObjCProperty(FirstProtocol, FirstModule,
1897+
SecondModule,
1898+
cast<ObjCPropertyDecl>(FirstDecl),
1899+
cast<ObjCPropertyDecl>(SecondDecl)))
1900+
return true;
1901+
break;
1902+
}
18171903
}
18181904

18191905
Diag(FirstDecl->getLocation(),

clang/lib/AST/ODRHash.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,15 @@ class ODRDeclVisitor : public ConstDeclVisitor<ODRDeclVisitor> {
337337
Inherited::VisitFieldDecl(D);
338338
}
339339

340+
void VisitObjCPropertyDecl(const ObjCPropertyDecl *D) {
341+
ID.AddInteger(D->getPropertyAttributes());
342+
ID.AddInteger(D->getPropertyImplementation());
343+
AddQualType(D->getType());
344+
AddDecl(D);
345+
346+
Inherited::VisitObjCPropertyDecl(D);
347+
}
348+
340349
void VisitFunctionDecl(const FunctionDecl *D) {
341350
// Handled by the ODRHash for FunctionDecl
342351
ID.AddInteger(D->getODRHash());
@@ -522,6 +531,7 @@ bool ODRHash::isSubDeclToBeProcessed(const Decl *D, const DeclContext *Parent) {
522531
case Decl::Typedef:
523532
case Decl::Var:
524533
case Decl::ObjCMethod:
534+
case Decl::ObjCProperty:
525535
return true;
526536
}
527537
}

clang/test/Modules/compare-objc-protocol.m

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,3 +242,108 @@ - (void)methodRequiredness;
242242
// expected-note@second.h:* {{but in 'Second' found 'required' method control}}
243243
id<CompareMethodRequirednessDefault> compareMethodRequirednessDefault; // no error
244244
#endif
245+
246+
#if defined(FIRST)
247+
@protocol CompareMatchingProperties
248+
@property int matchingPropName;
249+
@end
250+
251+
@protocol ComparePropertyPresence1
252+
@property int propPresence1;
253+
@end
254+
@protocol ComparePropertyPresence2
255+
@end
256+
257+
@protocol ComparePropertyName
258+
@property int propNameA;
259+
@end
260+
261+
@protocol ComparePropertyType
262+
@property int propType;
263+
@end
264+
265+
@protocol ComparePropertyOrder
266+
@property int propOrderX;
267+
@property int propOrderY;
268+
@end
269+
270+
@protocol CompareMatchingPropertyAttributes
271+
@property (nonatomic, assign) int matchingProp;
272+
@end
273+
@protocol ComparePropertyAttributes
274+
@property (nonatomic) int propAttributes;
275+
@end
276+
// Edge cases.
277+
@protocol CompareFirstImplAttribute
278+
@property int firstImplAttribute;
279+
@end
280+
@protocol CompareLastImplAttribute
281+
// Cannot test with protocols 'direct' attribute because it's not allowed.
282+
@property (class) int lastImplAttribute;
283+
@end
284+
#elif defined(SECOND)
285+
@protocol CompareMatchingProperties
286+
@property int matchingPropName;
287+
@end
288+
289+
@protocol ComparePropertyPresence1
290+
@end
291+
@protocol ComparePropertyPresence2
292+
@property int propPresence2;
293+
@end
294+
295+
@protocol ComparePropertyName
296+
@property int propNameB;
297+
@end
298+
299+
@protocol ComparePropertyType
300+
@property float propType;
301+
@end
302+
303+
@protocol ComparePropertyOrder
304+
@property int propOrderY;
305+
@property int propOrderX;
306+
@end
307+
308+
@protocol CompareMatchingPropertyAttributes
309+
@property (assign, nonatomic) int matchingProp;
310+
@end
311+
@protocol ComparePropertyAttributes
312+
@property (atomic) int propAttributes;
313+
@end
314+
// Edge cases.
315+
@protocol CompareFirstImplAttribute
316+
@property (readonly) int firstImplAttribute;
317+
@end
318+
@protocol CompareLastImplAttribute
319+
@property int lastImplAttribute;
320+
@end
321+
#else
322+
id<CompareMatchingProperties> compareMatchingProperties;
323+
id<ComparePropertyPresence1> comparePropertyPresence1;
324+
// expected-error@first.h:* {{'ComparePropertyPresence1' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property}}
325+
// expected-note@second.h:* {{but in 'Second' found end of class}}
326+
id<ComparePropertyPresence2> comparePropertyPresence2;
327+
// expected-error@first.h:* {{'ComparePropertyPresence2' has different definitions in different modules; first difference is definition in module 'First.Hidden' found end of class}}
328+
// expected-note@second.h:* {{but in 'Second' found property}}
329+
id<ComparePropertyName> comparePropertyName;
330+
// expected-error@first.h:* {{'ComparePropertyName' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'propNameA'}}
331+
// expected-note@second.h:* {{but in 'Second' found property 'propNameB'}}
332+
id<ComparePropertyType> comparePropertyType;
333+
// 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'}}
334+
// expected-note@second.h:* {{but in 'Second' found property 'propType' with type 'float'}}
335+
id<ComparePropertyOrder> comparePropertyOrder;
336+
// expected-error@first.h:* {{'ComparePropertyOrder' has different definitions in different modules; first difference is definition in module 'First.Hidden' found property 'propOrderX'}}
337+
// expected-note@second.h:* {{but in 'Second' found property 'propOrderY'}}
338+
339+
id<CompareMatchingPropertyAttributes> compareMatchingPropertyAttributes;
340+
id<ComparePropertyAttributes> comparePropertyAttributes;
341+
// 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}}
342+
// expected-note@second.h:* {{but in 'Second' found property 'propAttributes' with different 'nonatomic' attribute}}
343+
id<CompareFirstImplAttribute> compareFirstImplAttribute;
344+
// 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}}
345+
// expected-note@second.h:* {{but in 'Second' found property 'firstImplAttribute' with different 'readonly' attribute}}
346+
id<CompareLastImplAttribute> compareLastImplAttribute;
347+
// 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}}
348+
// expected-note@second.h:* {{but in 'Second' found property 'lastImplAttribute' with different 'class' attribute}}
349+
#endif

0 commit comments

Comments
 (0)