From a9d605647a013f1c894227b1ece7173c6967c07e Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Sat, 15 Jul 2017 02:55:13 +0000 Subject: [PATCH] [ODRHash] Revert r307743 which reverted r307720 Reapply r307720 to allow processing of constructors and destructors. Reuse the diagnostics for CXXMethodDecl for them. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@308077 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Basic/DiagnosticSerializationKinds.td | 70 +++++++++++------ lib/AST/ODRHash.cpp | 2 + lib/Serialization/ASTReader.cpp | 78 ++++++++++++------- test/Modules/odr_hash.cpp | 69 ++++++++++++++++ 4 files changed, 167 insertions(+), 52 deletions(-) diff --git a/include/clang/Basic/DiagnosticSerializationKinds.td b/include/clang/Basic/DiagnosticSerializationKinds.td index 0fc5484858..420ccebbfa 100644 --- a/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/include/clang/Basic/DiagnosticSerializationKinds.td @@ -147,18 +147,29 @@ def err_module_odr_violation_mismatch_decl_diff : Error< "%select{non-|}5mutable field %4|" "field %4 with %select{no|an}5 initalizer|" "field %4 with an initializer|" - "method %4|" - "method %4 is %select{not deleted|deleted}5|" - "method %4 is %select{|pure }5%select{not virtual|virtual}6|" - "method %4 is %select{not static|static}5|" - "method %4 is %select{not volatile|volatile}5|" - "method %4 is %select{not const|const}5|" - "method %4 is %select{not inline|inline}5|" - "method %4 that has %5 parameter%s5|" - "method %4 with %ordinal5 parameter of type %6%select{| decayed from %8}7|" - "method %4 with %ordinal5 parameter named %6|" - "method %4 with %ordinal5 parameter with%select{out|}6 a default argument|" - "method %4 with %ordinal5 parameter with a default argument|" + "%select{method %5|constructor|destructor}4|" + "%select{method %5|constructor|destructor}4 " + "is %select{not deleted|deleted}6|" + "%select{method %5|constructor|destructor}4 " + "is %select{|pure }6%select{not virtual|virtual}7|" + "%select{method %5|constructor|destructor}4 " + "is %select{not static|static}6|" + "%select{method %5|constructor|destructor}4 " + "is %select{not volatile|volatile}6|" + "%select{method %5|constructor|destructor}4 " + "is %select{not const|const}6|" + "%select{method %5|constructor|destructor}4 " + "is %select{not inline|inline}6|" + "%select{method %5|constructor|destructor}4 " + "that has %6 parameter%s6|" + "%select{method %5|constructor|destructor}4 " + "with %ordinal6 parameter of type %7%select{| decayed from %9}8|" + "%select{method %5|constructor|destructor}4 " + "with %ordinal6 parameter named %7|" + "%select{method %5|constructor|destructor}4 " + "with %ordinal6 parameter with%select{out|}7 a default argument|" + "%select{method %5|constructor|destructor}4 " + "with %ordinal6 parameter with a default argument|" "%select{typedef|type alias}4 name %5|" "%select{typedef|type alias}4 %5 with underlying type %6|" "data member with name %4|" @@ -183,18 +194,29 @@ def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found " "%select{non-|}3mutable field %2|" "field %2 with %select{no|an}3 initializer|" "field %2 with a different initializer|" - "method %2|" - "method %2 is %select{not deleted|deleted}3|" - "method %2 is %select{|pure }3%select{not virtual|virtual}4|" - "method %2 is %select{not static|static}3|" - "method %2 is %select{not volatile|volatile}3|" - "method %2 is %select{not const|const}3|" - "method %2 is %select{not inline|inline}3|" - "method %2 that has %3 parameter%s3|" - "method %2 with %ordinal3 parameter of type %4%select{| decayed from %6}5|" - "method %2 with %ordinal3 parameter named %4|" - "method %2 with %ordinal3 parameter with%select{out|}4 a default argument|" - "method %2 with %ordinal3 parameter with a different default argument|" + "%select{method %3|constructor|destructor}2|" + "%select{method %3|constructor|destructor}2 " + "is %select{not deleted|deleted}4|" + "%select{method %3|constructor|destructor}2 " + "is %select{|pure }4%select{not virtual|virtual}5|" + "%select{method %3|constructor|destructor}2 " + "is %select{not static|static}4|" + "%select{method %3|constructor|destructor}2 " + "is %select{not volatile|volatile}4|" + "%select{method %3|constructor|destructor}2 " + "is %select{not const|const}4|" + "%select{method %3|constructor|destructor}2 " + "is %select{not inline|inline}4|" + "%select{method %3|constructor|destructor}2 " + "that has %4 parameter%s4|" + "%select{method %3|constructor|destructor}2 " + "with %ordinal4 parameter of type %5%select{| decayed from %7}6|" + "%select{method %3|constructor|destructor}2 " + "with %ordinal4 parameter named %5|" + "%select{method %3|constructor|destructor}2 " + "with %ordinal4 parameter with%select{out|}5 a default argument|" + "%select{method %3|constructor|destructor}2 " + "with %ordinal4 parameter with a different default argument|" "%select{typedef|type alias}2 name %3|" "%select{typedef|type alias}2 %3 with different underlying type %4|" "data member with name %2|" diff --git a/lib/AST/ODRHash.cpp b/lib/AST/ODRHash.cpp index c16f4f336a..b19135384c 100644 --- a/lib/AST/ODRHash.cpp +++ b/lib/AST/ODRHash.cpp @@ -354,6 +354,8 @@ bool ODRHash::isWhitelistedDecl(const Decl *D, const CXXRecordDecl *Parent) { default: return false; case Decl::AccessSpec: + case Decl::CXXConstructor: + case Decl::CXXDestructor: case Decl::CXXMethod: case Decl::Field: case Decl::Friend: diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 678ecfc9a3..7da171b655 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -9341,6 +9341,8 @@ void ASTReader::diagnoseOdrViolations() { case Decl::Field: return Field; case Decl::CXXMethod: + case Decl::CXXConstructor: + case Decl::CXXDestructor: return CXXMethod; case Decl::TypeAlias: return TypeAlias; @@ -9669,17 +9671,30 @@ void ASTReader::diagnoseOdrViolations() { break; } case CXXMethod: { + enum { + DiagMethod, + DiagConstructor, + DiagDestructor, + } FirstMethodType, + SecondMethodType; + auto GetMethodTypeForDiagnostics = [](const CXXMethodDecl* D) { + if (isa(D)) return DiagConstructor; + if (isa(D)) return DiagDestructor; + return DiagMethod; + }; const CXXMethodDecl *FirstMethod = cast(FirstDecl); const CXXMethodDecl *SecondMethod = cast(SecondDecl); + FirstMethodType = GetMethodTypeForDiagnostics(FirstMethod); + SecondMethodType = GetMethodTypeForDiagnostics(SecondMethod); auto FirstName = FirstMethod->getDeclName(); auto SecondName = SecondMethod->getDeclName(); - if (FirstName != SecondName) { + if (FirstMethodType != SecondMethodType || FirstName != SecondName) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodName) - << FirstName; + << FirstMethodType << FirstName; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodName) - << SecondName; + << SecondMethodType << SecondName; Diagnosed = true; break; @@ -9690,11 +9705,11 @@ void ASTReader::diagnoseOdrViolations() { if (FirstDeleted != SecondDeleted) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodDeleted) - << FirstName << FirstDeleted; + << FirstMethodType << FirstName << FirstDeleted; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodDeleted) - << SecondName << SecondDeleted; + << SecondMethodType << SecondName << SecondDeleted; Diagnosed = true; break; } @@ -9707,10 +9722,10 @@ void ASTReader::diagnoseOdrViolations() { (FirstVirtual != SecondVirtual || FirstPure != SecondPure)) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodVirtual) - << FirstName << FirstPure << FirstVirtual; + << FirstMethodType << FirstName << FirstPure << FirstVirtual; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodVirtual) - << SecondName << SecondPure << SecondVirtual; + << SecondMethodType << SecondName << SecondPure << SecondVirtual; Diagnosed = true; break; } @@ -9725,10 +9740,10 @@ void ASTReader::diagnoseOdrViolations() { if (FirstStatic != SecondStatic) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodStatic) - << FirstName << FirstStatic; + << FirstMethodType << FirstName << FirstStatic; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodStatic) - << SecondName << SecondStatic; + << SecondMethodType << SecondName << SecondStatic; Diagnosed = true; break; } @@ -9738,10 +9753,10 @@ void ASTReader::diagnoseOdrViolations() { if (FirstVolatile != SecondVolatile) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodVolatile) - << FirstName << FirstVolatile; + << FirstMethodType << FirstName << FirstVolatile; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodVolatile) - << SecondName << SecondVolatile; + << SecondMethodType << SecondName << SecondVolatile; Diagnosed = true; break; } @@ -9751,10 +9766,10 @@ void ASTReader::diagnoseOdrViolations() { if (FirstConst != SecondConst) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodConst) - << FirstName << FirstConst; + << FirstMethodType << FirstName << FirstConst; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodConst) - << SecondName << SecondConst; + << SecondMethodType << SecondName << SecondConst; Diagnosed = true; break; } @@ -9764,10 +9779,10 @@ void ASTReader::diagnoseOdrViolations() { if (FirstInline != SecondInline) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodInline) - << FirstName << FirstInline; + << FirstMethodType << FirstName << FirstInline; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodInline) - << SecondName << SecondInline; + << SecondMethodType << SecondName << SecondInline; Diagnosed = true; break; } @@ -9777,10 +9792,10 @@ void ASTReader::diagnoseOdrViolations() { if (FirstNumParameters != SecondNumParameters) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodNumberParameters) - << FirstName << FirstNumParameters; + << FirstMethodType << FirstName << FirstNumParameters; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodNumberParameters) - << SecondName << SecondNumParameters; + << SecondMethodType << SecondName << SecondNumParameters; Diagnosed = true; break; } @@ -9800,24 +9815,27 @@ void ASTReader::diagnoseOdrViolations() { FirstParamType->getAs()) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodParameterType) - << FirstName << (I + 1) << FirstParamType << true - << ParamDecayedType->getOriginalType(); + << FirstMethodType << FirstName << (I + 1) << FirstParamType + << true << ParamDecayedType->getOriginalType(); } else { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodParameterType) - << FirstName << (I + 1) << FirstParamType << false; + << FirstMethodType << FirstName << (I + 1) << FirstParamType + << false; } if (const DecayedType *ParamDecayedType = SecondParamType->getAs()) { ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodParameterType) - << SecondName << (I + 1) << SecondParamType << true + << SecondMethodType << SecondName << (I + 1) + << SecondParamType << true << ParamDecayedType->getOriginalType(); } else { ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodParameterType) - << SecondName << (I + 1) << SecondParamType << false; + << SecondMethodType << SecondName << (I + 1) + << SecondParamType << false; } ParameterMismatch = true; break; @@ -9828,10 +9846,10 @@ void ASTReader::diagnoseOdrViolations() { if (FirstParamName != SecondParamName) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodParameterName) - << FirstName << (I + 1) << FirstParamName; + << FirstMethodType << FirstName << (I + 1) << FirstParamName; ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodParameterName) - << SecondName << (I + 1) << SecondParamName; + << SecondMethodType << SecondName << (I + 1) << SecondParamName; ParameterMismatch = true; break; } @@ -9842,12 +9860,14 @@ void ASTReader::diagnoseOdrViolations() { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodParameterSingleDefaultArgument) - << FirstName << (I + 1) << (FirstInit == nullptr) + << FirstMethodType << FirstName << (I + 1) + << (FirstInit == nullptr) << (FirstInit ? FirstInit->getSourceRange() : SourceRange()); ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodParameterSingleDefaultArgument) - << SecondName << (I + 1) << (SecondInit == nullptr) + << SecondMethodType << SecondName << (I + 1) + << (SecondInit == nullptr) << (SecondInit ? SecondInit->getSourceRange() : SourceRange()); ParameterMismatch = true; break; @@ -9858,11 +9878,13 @@ void ASTReader::diagnoseOdrViolations() { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodParameterDifferentDefaultArgument) - << FirstName << (I + 1) << FirstInit->getSourceRange(); + << FirstMethodType << FirstName << (I + 1) + << FirstInit->getSourceRange(); ODRDiagNote(SecondMethod->getLocation(), SecondMethod->getSourceRange(), MethodParameterDifferentDefaultArgument) - << SecondName << (I + 1) << SecondInit->getSourceRange(); + << SecondMethodType << SecondName << (I + 1) + << SecondInit->getSourceRange(); ParameterMismatch = true; break; diff --git a/test/Modules/odr_hash.cpp b/test/Modules/odr_hash.cpp index 91230c52c8..68a988dc8e 100644 --- a/test/Modules/odr_hash.cpp +++ b/test/Modules/odr_hash.cpp @@ -533,6 +533,75 @@ S15 s15; #endif } // namespace Method +namespace Constructor { +#if defined(FIRST) +struct S1 { + S1() {} + void foo() {} +}; +#elif defined(SECOND) +struct S1 { + void foo() {} + S1() {} +}; +#else +S1 s1; +// expected-error@second.h:* {{'Constructor::S1' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'foo'}} +// expected-note@first.h:* {{but in 'FirstModule' found constructor}} +#endif + +#if defined(FIRST) +struct S2 { + S2(int) {} + S2(int, int) {} +}; +#elif defined(SECOND) +struct S2 { + S2(int, int) {} + S2(int) {} +}; +#else +S2* s2; +// expected-error@second.h:* {{'Constructor::S2' has different definitions in different modules; first difference is definition in module 'SecondModule' found constructor that has 2 parameters}} +// expected-note@first.h:* {{but in 'FirstModule' found constructor that has 1 parameter}} +#endif +} // namespace Constructor + +namespace Destructor { +#if defined(FIRST) +struct S1 { + ~S1() {} + S1() {} +}; +#elif defined(SECOND) +struct S1 { + S1() {} + ~S1() {} +}; +#else +S1 s1; +// expected-error@second.h:* {{'Destructor::S1' has different definitions in different modules; first difference is definition in module 'SecondModule' found constructor}} +// expected-note@first.h:* {{but in 'FirstModule' found destructor}} +#endif + +#if defined(FIRST) +struct S2 { + virtual ~S2() {} + void foo() {} +}; +#elif defined(SECOND) +struct S2 { + ~S2() {} + virtual void foo() {} +}; +#else +S2 s2; +// expected-error@second.h:* {{'Destructor::S2' has different definitions in different modules; first difference is definition in module 'SecondModule' found destructor is not virtual}} +// expected-note@first.h:* {{but in 'FirstModule' found destructor is virtual}} +#endif + +} // namespace Destructor + // Naive parsing of AST can lead to cycles in processing. Ensure // self-references don't trigger an endless cycles of AST node processing. namespace SelfReference { -- 2.40.0