Skip to content

Commit

Permalink
[ODRHash] Add handling of TypedefType and DeclarationName
Browse files Browse the repository at this point in the history
Differential Revision: https://reviews.llvm.org/D21675

llvm-svn: 296078
  • Loading branch information
Weverything committed Feb 24, 2017
1 parent 8f34746 commit 8459ddf
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 3 deletions.
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/DiagnosticSerializationKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,14 @@ def err_module_odr_violation_mismatch_decl_diff : Error<
"static assert with condition|"
"static assert with message|"
"static assert with %select{|no }4message|"
"field %4}3">;
"field %4|field %4 with type %5}3">;

def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found "
"%select{"
"static assert with different condition|"
"static assert with different message|"
"static assert with %select{|no }2message|"
"field %2}1">;
"field %2|field %2 with type %3}1">;

def warn_module_uses_date_time : Warning<
"%select{precompiled header|module}0 uses __DATE__ or __TIME__">,
Expand Down
66 changes: 65 additions & 1 deletion clang/lib/AST/ODRHash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,57 @@ void ODRHash::AddIdentifierInfo(const IdentifierInfo *II) {
ID.AddString(II->getName());
}

void ODRHash::AddDeclarationName(DeclarationName Name) {
AddBoolean(Name.isEmpty());
if (Name.isEmpty())
return;

auto Kind = Name.getNameKind();
ID.AddInteger(Kind);
switch (Kind) {
case DeclarationName::Identifier:
AddIdentifierInfo(Name.getAsIdentifierInfo());
break;
case DeclarationName::ObjCZeroArgSelector:
case DeclarationName::ObjCOneArgSelector:
case DeclarationName::ObjCMultiArgSelector: {
Selector S = Name.getObjCSelector();
AddBoolean(S.isNull());
AddBoolean(S.isKeywordSelector());
AddBoolean(S.isUnarySelector());
unsigned NumArgs = S.getNumArgs();
for (unsigned i = 0; i < NumArgs; ++i) {
AddIdentifierInfo(S.getIdentifierInfoForSlot(i));
}
break;
}
case DeclarationName::CXXConstructorName:
case DeclarationName::CXXDestructorName:
AddQualType(Name.getCXXNameType());
break;
case DeclarationName::CXXOperatorName:
ID.AddInteger(Name.getCXXOverloadedOperator());
break;
case DeclarationName::CXXLiteralOperatorName:
AddIdentifierInfo(Name.getCXXLiteralIdentifier());
break;
case DeclarationName::CXXConversionFunctionName:
AddQualType(Name.getCXXNameType());
break;
case DeclarationName::CXXUsingDirective:
break;
case DeclarationName::CXXDeductionGuideName: {
auto *Template = Name.getCXXDeductionGuideTemplate();
AddBoolean(Template);
if (Template) {
AddDecl(Template);
}
}
}
}

void ODRHash::AddNestedNameSpecifier(const NestedNameSpecifier *NNS) {}
void ODRHash::AddTemplateName(TemplateName Name) {}
void ODRHash::AddDeclarationName(DeclarationName Name) {}
void ODRHash::AddTemplateArgument(TemplateArgument TA) {}
void ODRHash::AddTemplateParameterList(const TemplateParameterList *TPL) {}

Expand Down Expand Up @@ -192,6 +240,10 @@ void ODRHash::AddDecl(const Decl *D) {
}

ID.AddInteger(D->getKind());

if (const NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
AddDeclarationName(ND->getDeclName());
}
}

// Process a Type pointer. Add* methods call back into ODRHash while Visit*
Expand All @@ -212,6 +264,13 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
}
}

void AddDecl(Decl *D) {
Hash.AddBoolean(D);
if (D) {
Hash.AddDecl(D);
}
}

void Visit(const Type *T) {
ID.AddInteger(T->getTypeClass());
Inherited::Visit(T);
Expand All @@ -223,6 +282,11 @@ class ODRTypeVisitor : public TypeVisitor<ODRTypeVisitor> {
ID.AddInteger(T->getKind());
VisitType(T);
}

void VisitTypedefType(const TypedefType *T) {
AddDecl(T->getDecl());
VisitType(T);
}
};

void ODRHash::AddType(const Type *T) {
Expand Down
47 changes: 47 additions & 0 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9062,6 +9062,7 @@ void ASTReader::diagnoseOdrViolations() {
StaticAssertMessage,
StaticAssertOnlyMessage,
FieldName,
FieldTypeName,
};

// These lambdas have the common portions of the ODR diagnostics. This
Expand All @@ -9086,6 +9087,12 @@ void ASTReader::diagnoseOdrViolations() {
return Hash.CalculateHash();
};

auto ComputeDeclNameODRHash = [&Hash](const DeclarationName Name) {
Hash.clear();
Hash.AddDeclarationName(Name);
return Hash.CalculateHash();
};

switch (FirstDiffType) {
case Other:
case EndOfClass:
Expand Down Expand Up @@ -9166,6 +9173,46 @@ void ASTReader::diagnoseOdrViolations() {
Diagnosed = true;
break;
}

assert(
Context.hasSameType(FirstField->getType(), SecondField->getType()));

QualType FirstType = FirstField->getType();
QualType SecondType = SecondField->getType();
const TypedefType *FirstTypedef = dyn_cast<TypedefType>(FirstType);
const TypedefType *SecondTypedef = dyn_cast<TypedefType>(SecondType);

if ((FirstTypedef && !SecondTypedef) ||
(!FirstTypedef && SecondTypedef)) {
ODRDiagError(FirstField->getLocation(), FirstField->getSourceRange(),
FieldTypeName)
<< FirstII << FirstType;
ODRDiagNote(SecondField->getLocation(), SecondField->getSourceRange(),
FieldTypeName)
<< SecondII << SecondType;

Diagnosed = true;
break;
}

if (FirstTypedef && SecondTypedef) {
unsigned FirstHash = ComputeDeclNameODRHash(
FirstTypedef->getDecl()->getDeclName());
unsigned SecondHash = ComputeDeclNameODRHash(
SecondTypedef->getDecl()->getDeclName());
if (FirstHash != SecondHash) {
ODRDiagError(FirstField->getLocation(),
FirstField->getSourceRange(), FieldTypeName)
<< FirstII << FirstType;
ODRDiagNote(SecondField->getLocation(),
SecondField->getSourceRange(), FieldTypeName)
<< SecondII << SecondType;

Diagnosed = true;
break;
}
}

break;
}
}
Expand Down
42 changes: 42 additions & 0 deletions clang/test/Modules/odr_hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,36 @@ S3 s3;
// expected-error@first.h:* {{'Field::S3::x' from module 'FirstModule' is not present in definition of 'Field::S3' in module 'SecondModule'}}
// expected-note@second.h:* {{declaration of 'x' does not match}}
#endif

#if defined(FIRST)
typedef int A;
struct S4 {
A x;
};

struct S5 {
A x;
};
#elif defined(SECOND)
typedef int B;
struct S4 {
B x;
};

struct S5 {
int x;
};
#else
S4 s4;
// expected-error@second.h:* {{'Field::S4' has different definitions in different modules; first difference is definition in module 'SecondModule' found field 'x' with type 'B' (aka 'int')}}
// expected-note@first.h:* {{but in 'FirstModule' found field 'x' with type 'A' (aka 'int')}}

S5 s5;
// expected-error@second.h:* {{'Field::S5' has different definitions in different modules; first difference is definition in module 'SecondModule' found field 'x' with type 'int'}}
// expected-note@first.h:* {{but in 'FirstModule' found field 'x' with type 'A' (aka 'int')}}
#endif


} // namespace Field

// Naive parsing of AST can lead to cycles in processing. Ensure
Expand Down Expand Up @@ -193,6 +223,7 @@ struct NestedNamespaceSpecifier {};
// while struct T should error at the access specifier mismatch at the end.
namespace AllDecls {
#if defined(FIRST)
typedef int INT;
struct S {
public:
private:
Expand All @@ -203,8 +234,11 @@ struct S {

int x;
double y;

INT z;
};
#elif defined(SECOND)
typedef int INT;
struct S {
public:
private:
Expand All @@ -215,12 +249,15 @@ struct S {

int x;
double y;

INT z;
};
#else
S s;
#endif

#if defined(FIRST)
typedef int INT;
struct T {
public:
private:
Expand All @@ -232,9 +269,12 @@ struct T {
int x;
double y;

INT z;

private:
};
#elif defined(SECOND)
typedef int INT;
struct T {
public:
private:
Expand All @@ -246,6 +286,8 @@ struct T {
int x;
double y;

INT z;

public:
};
#else
Expand Down

0 comments on commit 8459ddf

Please sign in to comment.