Author: rtrieu
Date: Fri Sep 29 19:19:17 2017
New Revision: 314581
URL: http://llvm.org/viewvc/llvm-project?rev=314581&view=rev
Log:
[ODRHash] Add base classes to hashing CXXRecordDecl.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/AST/ODRHash.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/test/Modules/odr_hash.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=314581&r1=314580&r2=314581&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Fri Sep 29
19:19:17 2017
@@ -122,6 +122,27 @@ def note_second_module_difference : Note
def err_module_odr_violation_different_instantiations : Error<
"instantiation of %q0 is different in different modules">;
+def err_module_odr_violation_definition_data : Error <
+ "%q0 has different definitions in different modules; first difference is "
+ "%select{definition in module '%2'|defined here}1 found "
+ "%select{"
+ "%4 base %plural{1:class|:classes}4|"
+ "%4 virtual base %plural{1:class|:classes}4|"
+ "%ordinal4 base class with type %5|"
+ "%ordinal4 %select{non-virtual|virtual}5 base class %6|"
+ "%ordinal4 base class %5 with "
+ "%select{public|protected|private|no}6 access specifier}3">;
+
+def note_module_odr_violation_definition_data : Note <
+ "but in '%0' found "
+ "%select{"
+ "%2 base %plural{1:class|:classes}2|"
+ "%2 virtual base %plural{1:class|:classes}2|"
+ "%ordinal2 base class with different type %3|"
+ "%ordinal2 %select{non-virtual|virtual}3 base class %4|"
+ "%ordinal2 base class %3 with "
+ "%select{public|protected|private|no}4 access specifier}1">;
+
def err_module_odr_violation_template_parameter : Error <
"%q0 has different definitions in different modules; first difference is "
"%select{definition in module '%2'|defined here}1 found "
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=314581&r1=314580&r2=314581&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Sep 29 19:19:17 2017
@@ -1043,8 +1043,11 @@ private:
/// once recursing loading has been completed.
llvm::SmallVector<NamedDecl *, 16> PendingOdrMergeChecks;
+ using DataPointers =
+ std::pair<CXXRecordDecl *, struct CXXRecordDecl::DefinitionData *>;
+
/// \brief Record definitions in which we found an ODR violation.
- llvm::SmallDenseMap<CXXRecordDecl *, llvm::TinyPtrVector<CXXRecordDecl *>, 2>
+ llvm::SmallDenseMap<CXXRecordDecl *, llvm::SmallVector<DataPointers, 2>, 2>
PendingOdrMergeFailures;
/// \brief DeclContexts in which we have diagnosed an ODR violation.
Modified: cfe/trunk/lib/AST/ODRHash.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=314581&r1=314580&r2=314581&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ODRHash.cpp (original)
+++ cfe/trunk/lib/AST/ODRHash.cpp Fri Sep 29 19:19:17 2017
@@ -456,6 +456,14 @@ void ODRHash::AddCXXRecordDecl(const CXX
if (TD) {
AddTemplateParameterList(TD->getTemplateParameters());
}
+
+ ID.AddInteger(Record->getNumBases());
+ auto Bases = Record->bases();
+ for (auto Base : Bases) {
+ AddQualType(Base.getType());
+ ID.AddInteger(Base.isVirtual());
+ ID.AddInteger(Base.getAccessSpecifierAsWritten());
+ }
}
void ODRHash::AddDecl(const Decl *D) {
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=314581&r1=314580&r2=314581&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Sep 29 19:19:17 2017
@@ -9190,7 +9190,8 @@ void ASTReader::diagnoseOdrViolations()
Merge.first->decls_begin();
Merge.first->bases_begin();
Merge.first->vbases_begin();
- for (auto *RD : Merge.second) {
+ for (auto &RecordPair : Merge.second) {
+ auto *RD = RecordPair.first;
RD->decls_begin();
RD->bases_begin();
RD->vbases_begin();
@@ -9296,13 +9297,132 @@ void ASTReader::diagnoseOdrViolations()
bool Diagnosed = false;
CXXRecordDecl *FirstRecord = Merge.first;
std::string FirstModule = getOwningModuleNameForDiagnostic(FirstRecord);
- for (CXXRecordDecl *SecondRecord : Merge.second) {
+ for (auto &RecordPair : Merge.second) {
+ CXXRecordDecl *SecondRecord = RecordPair.first;
// Multiple different declarations got merged together; tell the user
// where they came from.
if (FirstRecord == SecondRecord)
continue;
std::string SecondModule =
getOwningModuleNameForDiagnostic(SecondRecord);
+
+ auto *FirstDD = FirstRecord->DefinitionData;
+ auto *SecondDD = RecordPair.second;
+
+ assert(FirstDD && SecondDD && "Definitions without DefinitionData");
+
+ // Diagnostics from DefinitionData are emitted here.
+ if (FirstDD != SecondDD) {
+ enum ODRDefinitionDataDifference {
+ NumBases,
+ NumVBases,
+ BaseType,
+ BaseVirtual,
+ BaseAccess,
+ };
+ auto ODRDiagError = [FirstRecord, &FirstModule,
+ this](SourceLocation Loc, SourceRange Range,
+ ODRDefinitionDataDifference DiffType) {
+ return Diag(Loc, diag::err_module_odr_violation_definition_data)
+ << FirstRecord << FirstModule.empty() << FirstModule << Range
+ << DiffType;
+ };
+ auto ODRDiagNote = [&SecondModule,
+ this](SourceLocation Loc, SourceRange Range,
+ ODRDefinitionDataDifference DiffType) {
+ return Diag(Loc, diag::note_module_odr_violation_definition_data)
+ << SecondModule << Range << DiffType;
+ };
+
+ ODRHash Hash;
+ auto ComputeQualTypeODRHash = [&Hash](QualType Ty) {
+ Hash.clear();
+ Hash.AddQualType(Ty);
+ return Hash.CalculateHash();
+ };
+
+ unsigned FirstNumBases = FirstDD->NumBases;
+ unsigned FirstNumVBases = FirstDD->NumVBases;
+ unsigned SecondNumBases = SecondDD->NumBases;
+ unsigned SecondNumVBases = SecondDD->NumVBases;
+
+ auto GetSourceRange = [](struct CXXRecordDecl::DefinitionData *DD) {
+ unsigned NumBases = DD->NumBases;
+ if (NumBases == 0) return SourceRange();
+ auto bases = DD->bases();
+ return SourceRange(bases[0].getLocStart(),
+ bases[NumBases - 1].getLocEnd());
+ };
+
+ if (FirstNumBases != SecondNumBases) {
+ ODRDiagError(FirstRecord->getLocation(), GetSourceRange(FirstDD),
+ NumBases)
+ << FirstNumBases;
+ ODRDiagNote(SecondRecord->getLocation(), GetSourceRange(SecondDD),
+ NumBases)
+ << SecondNumBases;
+ Diagnosed = true;
+ break;
+ }
+
+ if (FirstNumVBases != SecondNumVBases) {
+ ODRDiagError(FirstRecord->getLocation(), GetSourceRange(FirstDD),
+ NumVBases)
+ << FirstNumVBases;
+ ODRDiagNote(SecondRecord->getLocation(), GetSourceRange(SecondDD),
+ NumVBases)
+ << SecondNumVBases;
+ Diagnosed = true;
+ break;
+ }
+
+ auto FirstBases = FirstDD->bases();
+ auto SecondBases = SecondDD->bases();
+ unsigned i = 0;
+ for (i = 0; i < FirstNumBases; ++i) {
+ auto FirstBase = FirstBases[i];
+ auto SecondBase = SecondBases[i];
+ if (ComputeQualTypeODRHash(FirstBase.getType()) !=
+ ComputeQualTypeODRHash(SecondBase.getType())) {
+ ODRDiagError(FirstRecord->getLocation(),
FirstBase.getSourceRange(),
+ BaseType)
+ << (i + 1) << FirstBase.getType();
+ ODRDiagNote(SecondRecord->getLocation(),
+ SecondBase.getSourceRange(), BaseType)
+ << (i + 1) << SecondBase.getType();
+ break;
+ }
+
+ if (FirstBase.isVirtual() != SecondBase.isVirtual()) {
+ ODRDiagError(FirstRecord->getLocation(),
FirstBase.getSourceRange(),
+ BaseVirtual)
+ << (i + 1) << FirstBase.isVirtual() << FirstBase.getType();
+ ODRDiagNote(SecondRecord->getLocation(),
+ SecondBase.getSourceRange(), BaseVirtual)
+ << (i + 1) << SecondBase.isVirtual() << SecondBase.getType();
+ break;
+ }
+
+ if (FirstBase.getAccessSpecifierAsWritten() !=
+ SecondBase.getAccessSpecifierAsWritten()) {
+ ODRDiagError(FirstRecord->getLocation(),
FirstBase.getSourceRange(),
+ BaseAccess)
+ << (i + 1) << FirstBase.getType()
+ << (int)FirstBase.getAccessSpecifierAsWritten();
+ ODRDiagNote(SecondRecord->getLocation(),
+ SecondBase.getSourceRange(), BaseAccess)
+ << (i + 1) << SecondBase.getType()
+ << (int)SecondBase.getAccessSpecifierAsWritten();
+ break;
+ }
+ }
+
+ if (i != FirstNumBases) {
+ Diagnosed = true;
+ break;
+ }
+ }
+
using DeclHashes = llvm::SmallVector<std::pair<Decl *, unsigned>, 4>;
const ClassTemplateDecl *FirstTemplate =
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=314581&r1=314580&r2=314581&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Sep 29 19:19:17 2017
@@ -1755,7 +1755,8 @@ void ASTDeclReader::MergeDefinitionData(
}
if (DetectedOdrViolation)
-
Reader.PendingOdrMergeFailures[DD.Definition].push_back(MergeDD.Definition);
+ Reader.PendingOdrMergeFailures[DD.Definition].push_back(
+ {MergeDD.Definition, &MergeDD});
}
void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update) {
Modified: cfe/trunk/test/Modules/odr_hash.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=314581&r1=314580&r2=314581&view=diff
==============================================================================
--- cfe/trunk/test/Modules/odr_hash.cpp (original)
+++ cfe/trunk/test/Modules/odr_hash.cpp Fri Sep 29 19:19:17 2017
@@ -1577,6 +1577,126 @@ using TemplateParameters::S6;
#endif
} // namespace TemplateParameters
+namespace BaseClass {
+#if defined(FIRST)
+struct B1 {};
+struct S1 : B1 {};
+#elif defined(SECOND)
+struct S1 {};
+#else
+S1 s1;
+// expected-error@second.h:* {{'BaseClass::S1' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 0 base classes}}
+// expected-note@first.h:* {{but in 'FirstModule' found 1 base class}}
+#endif
+
+#if defined(FIRST)
+struct S2 {};
+#elif defined(SECOND)
+struct B2 {};
+struct S2 : virtual B2 {};
+#else
+S2 s2;
+// expected-error@second.h:* {{'BaseClass::S2' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 1 base class}}
+// expected-note@first.h:* {{but in 'FirstModule' found 0 base classes}}
+#endif
+
+#if defined(FIRST)
+struct B3a {};
+struct S3 : B3a {};
+#elif defined(SECOND)
+struct B3b {};
+struct S3 : virtual B3b {};
+#else
+S3 s3;
+// expected-error@second.h:* {{'BaseClass::S3' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 1 virtual base class}}
+// expected-note@first.h:* {{but in 'FirstModule' found 0 virtual base
classes}}
+#endif
+
+#if defined(FIRST)
+struct B4a {};
+struct S4 : B4a {};
+#elif defined(SECOND)
+struct B4b {};
+struct S4 : B4b {};
+#else
+S4 s4;
+// expected-error@second.h:* {{'BaseClass::S4' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 1st base class with type 'BaseClass::B4b'}}
+// expected-note@first.h:* {{but in 'FirstModule' found 1st base class with
different type 'BaseClass::B4a'}}
+#endif
+
+#if defined(FIRST)
+struct B5a {};
+struct S5 : virtual B5a {};
+#elif defined(SECOND)
+struct B5a {};
+struct S5 : B5a {};
+#else
+S5 s5;
+// expected-error@second.h:* {{'BaseClass::S5' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 0 virtual base classes}}
+// expected-note@first.h:* {{but in 'FirstModule' found 1 virtual base class}}
+#endif
+
+#if defined(FIRST)
+struct B6a {};
+struct S6 : B6a {};
+#elif defined(SECOND)
+struct B6a {};
+struct S6 : virtual B6a {};
+#else
+S6 s6;
+// expected-error@second.h:* {{'BaseClass::S6' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 1 virtual base class}}
+// expected-note@first.h:* {{but in 'FirstModule' found 0 virtual base
classes}}
+#endif
+
+#if defined(FIRST)
+struct B7a {};
+struct S7 : protected B7a {};
+#elif defined(SECOND)
+struct B7a {};
+struct S7 : B7a {};
+#else
+S7 s7;
+// expected-error@second.h:* {{'BaseClass::S7' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 1st base class 'BaseClass::B7a' with no access specifier}}
+// expected-note@first.h:* {{but in 'FirstModule' found 1st base class
'BaseClass::B7a' with protected access specifier}}
+#endif
+
+#if defined(FIRST)
+struct B8a {};
+struct S8 : public B8a {};
+#elif defined(SECOND)
+struct B8a {};
+struct S8 : private B8a {};
+#else
+S8 s8;
+// expected-error@second.h:* {{'BaseClass::S8' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 1st base class 'BaseClass::B8a' with private access specifier}}
+// expected-note@first.h:* {{but in 'FirstModule' found 1st base class
'BaseClass::B8a' with public access specifier}}
+#endif
+
+#if defined(FIRST)
+struct B9a {};
+struct S9 : private B9a {};
+#elif defined(SECOND)
+struct B9a {};
+struct S9 : public B9a {};
+#else
+S9 s9;
+// expected-error@second.h:* {{'BaseClass::S9' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 1st base class 'BaseClass::B9a' with public access specifier}}
+// expected-note@first.h:* {{but in 'FirstModule' found 1st base class
'BaseClass::B9a' with private access specifier}}
+#endif
+
+#if defined(FIRST)
+struct B10a {};
+struct S10 : B10a {};
+#elif defined(SECOND)
+struct B10a {};
+struct S10 : protected B10a {};
+#else
+S10 s10;
+// expected-error@second.h:* {{'BaseClass::S10' has different definitions in
different modules; first difference is definition in module 'SecondModule'
found 1st base class 'BaseClass::B10a' with protected access specifier}}
+// expected-note@first.h:* {{but in 'FirstModule' found 1st base class
'BaseClass::B10a' with no access specifier}}
+#endif
+} // namespace BaseClass
+
// Interesting cases that should not cause errors. struct S should not error
// while struct T should error at the access specifier mismatch at the end.
namespace AllDecls {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
