From: Richard Trieu Date: Tue, 10 Jul 2018 01:40:50 +0000 (+0000) Subject: [ODRHash] Merge the two function hashes into one. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=27c1b1a638d2f6300ebc6f64fd97bd6f4d9c9e2d;p=platform%2Fupstream%2Fllvm.git [ODRHash] Merge the two function hashes into one. Functions that are a sub-Decl of a record were hashed differently than other functions. This change keeps the AddFunctionDecl function and the hash of records now calls this function. In addition, AddFunctionDecl has an option to perform a hash as if the body was absent, which is required for some checks after loading modules. Additional logic prevents multiple error message from being printed. llvm-svn: 336632 --- diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index ed0725d..e419ded 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2504,6 +2504,10 @@ public: /// stored on first call, then the stored value returned on the other calls. unsigned getODRHash(); + /// Returns cached ODRHash of the function. This must have been previously + /// computed and stored. + unsigned getODRHash() const; + // Implement isa/cast/dyncast/etc. static bool classof(const Decl *D) { return classofKind(D->getKind()); } static bool classofKind(Kind K) { diff --git a/clang/include/clang/AST/ODRHash.h b/clang/include/clang/AST/ODRHash.h index d4cddb8..6a66ba1 100644 --- a/clang/include/clang/AST/ODRHash.h +++ b/clang/include/clang/AST/ODRHash.h @@ -54,8 +54,9 @@ public: void AddCXXRecordDecl(const CXXRecordDecl *Record); // Use this for ODR checking functions between modules. This method compares - // more information than the AddDecl class. - void AddFunctionDecl(const FunctionDecl *Function); + // more information than the AddDecl class. SkipBody will process the + // hash as if the function has no body. + void AddFunctionDecl(const FunctionDecl *Function, bool SkipBody = false); // Process SubDecls of the main Decl. This method calls the DeclVisitor // while AddDecl does not. diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index 4f8f27c..a70e48a 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -192,6 +192,8 @@ def err_module_odr_violation_mismatch_decl_diff : Error< "%select{method %5|constructor|destructor}4 " "is %select{not deleted|deleted}6|" "%select{method %5|constructor|destructor}4 " + "is %select{not defaulted|defaulted}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|" @@ -217,6 +219,10 @@ def err_module_odr_violation_mismatch_decl_diff : Error< "with %6 template argument%s6|" "%select{method %5|constructor|destructor}4 " "with %6 for %ordinal7 template argument|" + "%select{method %5|constructor|destructor}4 " + "with %select{no body|body}6|" + "%select{method %5|constructor|destructor}4 " + "with body|" "%select{typedef|type alias}4 name %5|" "%select{typedef|type alias}4 %5 with underlying type %6|" "data member with name %4|" @@ -257,6 +263,8 @@ def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found " "%select{method %3|constructor|destructor}2 " "is %select{not deleted|deleted}4|" "%select{method %3|constructor|destructor}2 " + "is %select{not defaulted|defaulted}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|" @@ -282,6 +290,10 @@ def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found " "with %4 template argument%s4|" "%select{method %3|constructor|destructor}2 " "with %4 for %ordinal5 template argument|" + "%select{method %3|constructor|destructor}2 " + "with %select{no body|body}4|" + "%select{method %3|constructor|destructor}2 " + "with different body|" "%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/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 08718e0..7db105e 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3658,18 +3658,15 @@ unsigned FunctionDecl::getMemoryFunctionKind() const { return 0; } +unsigned FunctionDecl::getODRHash() const { + assert(HasODRHash); + return ODRHash; +} + unsigned FunctionDecl::getODRHash() { if (HasODRHash) return ODRHash; - if (FunctionDecl *Definition = getDefinition()) { - if (Definition != this) { - HasODRHash = true; - ODRHash = Definition->getODRHash(); - return ODRHash; - } - } - if (auto *FT = getInstantiatedFromMemberFunction()) { HasODRHash = true; ODRHash = FT->getODRHash(); diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp index ef1235e..45b8ee6 100644 --- a/clang/lib/AST/ODRHash.cpp +++ b/clang/lib/AST/ODRHash.cpp @@ -317,35 +317,14 @@ public: } void VisitFunctionDecl(const FunctionDecl *D) { - ID.AddInteger(D->getStorageClass()); - Hash.AddBoolean(D->isInlineSpecified()); - Hash.AddBoolean(D->isVirtualAsWritten()); - Hash.AddBoolean(D->isPure()); - Hash.AddBoolean(D->isDeletedAsWritten()); - - ID.AddInteger(D->param_size()); - - for (auto *Param : D->parameters()) { - Hash.AddSubDecl(Param); - } - - AddQualType(D->getReturnType()); - - const auto* SpecializationArgs = D->getTemplateSpecializationArgs(); - Hash.AddBoolean(SpecializationArgs); - if (SpecializationArgs) { - ID.AddInteger(SpecializationArgs->size()); - for (const TemplateArgument &TA : SpecializationArgs->asArray()) { - Hash.AddTemplateArgument(TA); - } - } + // Handled by the ODRHash for FunctionDecl + ID.AddInteger(D->getODRHash()); Inherited::VisitFunctionDecl(D); } void VisitCXXMethodDecl(const CXXMethodDecl *D) { - Hash.AddBoolean(D->isConst()); - Hash.AddBoolean(D->isVolatile()); + // Handled by the ODRHash for FunctionDecl Inherited::VisitCXXMethodDecl(D); } @@ -425,7 +404,6 @@ public: } void VisitFunctionTemplateDecl(const FunctionTemplateDecl *D) { - Visit(D->getTemplatedDecl()); AddDecl(D->getTemplatedDecl()); Inherited::VisitFunctionTemplateDecl(D); } @@ -479,9 +457,13 @@ void ODRHash::AddCXXRecordDecl(const CXXRecordDecl *Record) { // Filter out sub-Decls which will not be processed in order to get an // accurate count of Decl's. llvm::SmallVector Decls; - for (const Decl *SubDecl : Record->decls()) { + for (Decl *SubDecl : Record->decls()) { if (isWhitelistedDecl(SubDecl, Record)) { Decls.push_back(SubDecl); + if (auto *Function = dyn_cast(SubDecl)) { + // Compute/Preload ODRHash into FunctionDecl. + Function->getODRHash(); + } } } @@ -505,25 +487,48 @@ void ODRHash::AddCXXRecordDecl(const CXXRecordDecl *Record) { } } -void ODRHash::AddFunctionDecl(const FunctionDecl *Function) { +void ODRHash::AddFunctionDecl(const FunctionDecl *Function, + bool SkipBody) { assert(Function && "Expecting non-null pointer."); - // Skip hashing these kinds of function. - if (Function->isImplicit()) return; - if (Function->isDefaulted()) return; - if (Function->isDeleted()) return; - if (!Function->hasBody()) return; - if (!Function->getBody()) return; - // Skip functions that are specializations or in specialization context. const DeclContext *DC = Function; while (DC) { if (isa(DC)) return; - if (auto *F = dyn_cast(DC)) - if (F->isFunctionTemplateSpecialization()) return; + if (auto *F = dyn_cast(DC)) { + if (F->isFunctionTemplateSpecialization()) { + if (!isa(DC)) return; + if (DC->getLexicalParent()->isFileContext()) return; + // Inline method specializations are the only supported + // specialization for now. + } + } DC = DC->getParent(); } + ID.AddInteger(Function->getDeclKind()); + + const auto *SpecializationArgs = Function->getTemplateSpecializationArgs(); + AddBoolean(SpecializationArgs); + if (SpecializationArgs) { + ID.AddInteger(SpecializationArgs->size()); + for (const TemplateArgument &TA : SpecializationArgs->asArray()) { + AddTemplateArgument(TA); + } + } + + if (const auto *Method = dyn_cast(Function)) { + AddBoolean(Method->isConst()); + AddBoolean(Method->isVolatile()); + } + + ID.AddInteger(Function->getStorageClass()); + AddBoolean(Function->isInlineSpecified()); + AddBoolean(Function->isVirtualAsWritten()); + AddBoolean(Function->isPure()); + AddBoolean(Function->isDeletedAsWritten()); + AddBoolean(Function->isExplicitlyDefaulted()); + AddDecl(Function); AddQualType(Function->getReturnType()); @@ -532,7 +537,21 @@ void ODRHash::AddFunctionDecl(const FunctionDecl *Function) { for (auto Param : Function->parameters()) AddSubDecl(Param); - AddStmt(Function->getBody()); + if (SkipBody) { + AddBoolean(false); + return; + } + + const bool HasBody = Function->isThisDeclarationADefinition() && + !Function->isDefaulted() && !Function->isDeleted() && + !Function->isLateTemplateParsed(); + AddBoolean(HasBody); + if (HasBody) { + auto *Body = Function->getBody(); + AddBoolean(Body); + if (Body) + AddStmt(Body); + } } void ODRHash::AddDecl(const Decl *D) { diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 84286ca..3277243 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9397,7 +9397,15 @@ void ASTReader::finishPendingActions() { if (!FD->isLateTemplateParsed() && !NonConstDefn->isLateTemplateParsed() && FD->getODRHash() != NonConstDefn->getODRHash()) { - PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn); + if (!isa(FD)) { + PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn); + } else if (FD->getLexicalParent()->isFileContext() && + NonConstDefn->getLexicalParent()->isFileContext()) { + // Only diagnose out-of-line method definitions. If they are + // in class definitions, then an error will be generated when + // processing the class bodies. + PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn); + } } } continue; @@ -10069,6 +10077,7 @@ void ASTReader::diagnoseOdrViolations() { FieldDifferentInitializers, MethodName, MethodDeleted, + MethodDefaulted, MethodVirtual, MethodStatic, MethodVolatile, @@ -10082,6 +10091,8 @@ void ASTReader::diagnoseOdrViolations() { MethodNoTemplateArguments, MethodDifferentNumberTemplateArguments, MethodDifferentTemplateArgument, + MethodSingleBody, + MethodDifferentBody, TypedefName, TypedefType, VarName, @@ -10315,8 +10326,8 @@ void ASTReader::diagnoseOdrViolations() { break; } - const bool FirstDeleted = FirstMethod->isDeleted(); - const bool SecondDeleted = SecondMethod->isDeleted(); + const bool FirstDeleted = FirstMethod->isDeletedAsWritten(); + const bool SecondDeleted = SecondMethod->isDeletedAsWritten(); if (FirstDeleted != SecondDeleted) { ODRDiagError(FirstMethod->getLocation(), FirstMethod->getSourceRange(), MethodDeleted) @@ -10329,6 +10340,20 @@ void ASTReader::diagnoseOdrViolations() { break; } + const bool FirstDefaulted = FirstMethod->isExplicitlyDefaulted(); + const bool SecondDefaulted = SecondMethod->isExplicitlyDefaulted(); + if (FirstDefaulted != SecondDefaulted) { + ODRDiagError(FirstMethod->getLocation(), + FirstMethod->getSourceRange(), MethodDefaulted) + << FirstMethodType << FirstName << FirstDefaulted; + + ODRDiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), MethodDefaulted) + << SecondMethodType << SecondName << SecondDefaulted; + Diagnosed = true; + break; + } + const bool FirstVirtual = FirstMethod->isVirtualAsWritten(); const bool SecondVirtual = SecondMethod->isVirtualAsWritten(); const bool FirstPure = FirstMethod->isPure(); @@ -10594,6 +10619,44 @@ void ASTReader::diagnoseOdrViolations() { break; } } + + // Compute the hash of the method as if it has no body. + auto ComputeCXXMethodODRHash = [&Hash](const CXXMethodDecl *D) { + Hash.clear(); + Hash.AddFunctionDecl(D, true /*SkipBody*/); + return Hash.CalculateHash(); + }; + + // Compare the hash generated to the hash stored. A difference means + // that a body was present in the original source. Due to merging, + // the stardard way of detecting a body will not work. + const bool HasFirstBody = + ComputeCXXMethodODRHash(FirstMethod) != FirstMethod->getODRHash(); + const bool HasSecondBody = + ComputeCXXMethodODRHash(SecondMethod) != SecondMethod->getODRHash(); + + if (HasFirstBody != HasSecondBody) { + ODRDiagError(FirstMethod->getLocation(), + FirstMethod->getSourceRange(), MethodSingleBody) + << FirstMethodType << FirstName << HasFirstBody; + ODRDiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), MethodSingleBody) + << SecondMethodType << SecondName << HasSecondBody; + Diagnosed = true; + break; + } + + if (HasFirstBody && HasSecondBody) { + ODRDiagError(FirstMethod->getLocation(), + FirstMethod->getSourceRange(), MethodDifferentBody) + << FirstMethodType << FirstName; + ODRDiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), MethodDifferentBody) + << SecondMethodType << SecondName; + Diagnosed = true; + break; + } + break; } case TypeAlias: diff --git a/clang/test/Modules/odr_hash.cpp b/clang/test/Modules/odr_hash.cpp index 9cb177f..28ee423 100644 --- a/clang/test/Modules/odr_hash.cpp +++ b/clang/test/Modules/odr_hash.cpp @@ -660,6 +660,202 @@ Invalid1* i1; #undef DECLS } // namespace Method +namespace MethodBody { +#if defined(FIRST) +struct S1 { + int A() { return 0; } +}; +#elif defined(SECOND) +struct S1 { + int A() { return 0; } +}; +#else +S1 s1; +#endif + +#if defined(FIRST) +struct S2 { + int BothBodies() { return 0; } +}; +#elif defined(SECOND) +struct S2 { + int BothBodies() { return 1; } +}; +#else +S2 s2; +// expected-error@first.h:* {{'MethodBody::S2' has different definitions in different modules; first difference is definition in module 'FirstModule' found method 'BothBodies' with body}} +// expected-note@second.h:* {{but in 'SecondModule' found method 'BothBodies' with different body}} +#endif + +#if defined(FIRST) +struct S3 { + int FirstBody() { return 0; } +}; +#elif defined(SECOND) +struct S3 { + int FirstBody(); +}; +#else +S3 s3; +// expected-error@first.h:* {{'MethodBody::S3' has different definitions in different modules; first difference is definition in module 'FirstModule' found method 'FirstBody' with body}} +// expected-note@second.h:* {{but in 'SecondModule' found method 'FirstBody' with no body}} +#endif + +#if defined(FIRST) +struct S4 { + int SecondBody(); +}; +#elif defined(SECOND) +struct S4 { + int SecondBody() { return 0; } +}; +#else +S4 s4; +// expected-error@first.h:* {{'MethodBody::S4' has different definitions in different modules; first difference is definition in module 'FirstModule' found method 'SecondBody' with no body}} +// expected-note@second.h:* {{but in 'SecondModule' found method 'SecondBody' with body}} +#endif + +#if defined(FIRST) +struct S5 { + int FirstBodySecondOutOfLine() { return 0; } +}; +#elif defined(SECOND) +struct S5 { + int FirstBodySecondOutOfLine(); +}; +int S5::FirstBodySecondOutOfLine() { return 0; } +#else +S5 s5; +// expected-error@second.h:* {{'MethodBody::S5' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'FirstBodySecondOutOfLine' with no body}} +// expected-note@first.h:* {{but in 'FirstModule' found method 'FirstBodySecondOutOfLine' with body}} +#endif + +#if defined(FIRST) +struct S6 { + int FirstOutOfLineSecondBody(); +}; +int S6::FirstOutOfLineSecondBody() { return 0; } +#elif defined(SECOND) +struct S6 { + int FirstOutOfLineSecondBody() { return 0; } +}; +#else +S6 s6; +// expected-error@first.h:* {{'MethodBody::S6' has different definitions in different modules; first difference is definition in module 'FirstModule' found method 'FirstOutOfLineSecondBody' with no body}} +// expected-note@second.h:* {{but in 'SecondModule' found method 'FirstOutOfLineSecondBody' with body}} +#endif + +#if defined(FIRST) +struct S7 { + int BothOutOfLine(); +}; +int S7::BothOutOfLine() { return 1; } +#elif defined(SECOND) +struct S7 { + int BothOutOfLine(); +}; +int S7::BothOutOfLine() { return 0; } +#else +S7 s7; +// expected-error@second.h:* {{'MethodBody::S7::BothOutOfLine' has different definitions in different modules; definition in module 'SecondModule' first difference is function body}} +// expected-note@first.h:* {{but in 'FirstModule' found a different body}} +#endif + +#if defined(FIRST) +struct S8 { + int FirstBodySecondOutOfLine() { return 0; } +}; +#elif defined(SECOND) +struct S8 { + int FirstBodySecondOutOfLine(); +}; +int S8::FirstBodySecondOutOfLine() { return 1; } +#else +S8 s8; +// expected-error@second.h:* {{'MethodBody::S8' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'FirstBodySecondOutOfLine' with no body}} +// expected-note@first.h:* {{but in 'FirstModule' found method 'FirstBodySecondOutOfLine' with body}} +#endif + +#if defined(FIRST) +struct S9 { + int FirstOutOfLineSecondBody(); +}; +int S9::FirstOutOfLineSecondBody() { return 1; } +#elif defined(SECOND) +struct S9 { + int FirstOutOfLineSecondBody() { return 0; } +}; +#else +S9 s9; +// expected-error@first.h:* {{'MethodBody::S9' has different definitions in different modules; first difference is definition in module 'FirstModule' found method 'FirstOutOfLineSecondBody' with no body}} +// expected-note@second.h:* {{but in 'SecondModule' found method 'FirstOutOfLineSecondBody' with body}} +#endif + +#if defined(FIRST) +struct S10 { + S10(int); + S10() = delete; +}; +#elif defined(SECOND) +struct S10 { + S10(int); + S10(); +}; +#else +S10 s10(10); +// expected-error@first.h:* {{'MethodBody::S10' has different definitions in different modules; first difference is definition in module 'FirstModule' found constructor is deleted}} +// expected-note@second.h:* {{but in 'SecondModule' found constructor is not deleted}} +#endif + +#if defined(FIRST) +struct S11 { + S11() = default; +}; +#elif defined(SECOND) +struct S11 { + S11(); +}; +#else +S11 s11; +// expected-error@first.h:* {{'MethodBody::S11' has different definitions in different modules; first difference is definition in module 'FirstModule' found constructor is defaulted}} +// expected-note@second.h:* {{but in 'SecondModule' found constructor is not defaulted}} +#endif + +#define DECLS(CLASSNAME) \ + CLASSNAME() = default; \ + ~CLASSNAME() = delete; \ + void A(); \ + void B() { return; }; \ + void C(); \ + void D(); + +#define OUTOFLINEDEFS(CLASSNAME) \ + void CLASSNAME::C() {} \ + void CLASSNAME::D() { return; } + +#if defined(FIRST) || defined(SECOND) +struct Valid1 { + DECLS(Valid1) +}; +OUTOFLINEDEFS(Valid1) +#else +Valid1* v1; +#endif + +#if defined(FIRST) || defined(SECOND) +struct Invalid1 { + DECLS(Invalid1) + ACCESS +}; +OUTOFLINEDEFS(Invalid1) +#else +Invalid1* i1; +// expected-error@first.h:* {{'MethodBody::Invalid1' has different definitions in different modules; first difference is definition in module 'FirstModule' found public access specifier}} +// expected-note@second.h:* {{but in 'SecondModule' found private access specifier}} +#endif +#undef DECLS +} // namespace MethodBody + namespace Constructor { #if defined(FIRST) struct S1 { @@ -3631,6 +3827,32 @@ template void S11::foo() {} S11 s11; #endif +#if defined(FIRST) +struct S12 { + void foo(int x); +}; +#elif defined(SECOND) +struct S12 { + void foo(int x); +}; +void S12::foo(int y) {} +#else +S12 s12; +#endif + +#if defined(FIRST) +struct S13 { + void foo(int x); +}; +void S13::foo(int y) {} +#elif defined(SECOND) +struct S13 { + void foo(int x); +}; +void S13::foo(int y) {} +#else +S13 s13; +#endif } // namespace FunctionDecl namespace DeclTemplateArguments {