diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index 4af4c18ced33..07ecf755ad53 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -146,7 +146,12 @@ def err_module_odr_violation_mismatch_decl_diff : Error< "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}3">; + "method %4 is %select{not inline|inline}5|" + "method %4 that has %5 parameter%s5|" + "method %4 with %ordinal5 parameter of type %6|" + "method %4 with %ordinal5 parameter named %6|" + "method %4 with %ordinal5 parameter with %select{no |}6default argument|" + "method %4 with %ordinal5 parameter with default argument}3">; def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found " "%select{" @@ -166,7 +171,12 @@ def note_module_odr_violation_mismatch_decl_diff : Note<"but in '%0' found " "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}1">; + "method %2 is %select{not inline|inline}3|" + "method %2 that has %3 parameter%s3|" + "method %2 with %ordinal3 parameter of type %4|" + "method %2 with %ordinal3 parameter named %4|" + "method %2 with %ordinal3 parameter with %select{no |}4default argument|" + "method %2 with %ordinal3 parameter with different default argument}1">; def warn_module_uses_date_time : Warning< "%select{precompiled header|module}0 uses __DATE__ or __TIME__">, diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp index d72eebbe8e48..1b323e4b759b 100644 --- a/clang/lib/AST/ODRHash.cpp +++ b/clang/lib/AST/ODRHash.cpp @@ -169,6 +169,11 @@ public: Inherited::VisitValueDecl(D); } + void VisitParmVarDecl(const ParmVarDecl *D) { + AddStmt(D->getDefaultArg()); + Inherited::VisitParmVarDecl(D); + } + void VisitAccessSpecDecl(const AccessSpecDecl *D) { ID.AddInteger(D->getAccess()); Inherited::VisitAccessSpecDecl(D); @@ -202,6 +207,12 @@ public: Hash.AddBoolean(D->isPure()); Hash.AddBoolean(D->isDeletedAsWritten()); + ID.AddInteger(D->param_size()); + + for (auto *Param : D->parameters()) { + Hash.AddSubDecl(Param); + } + Inherited::VisitFunctionDecl(D); } @@ -315,6 +326,10 @@ public: } } + void AddQualType(QualType T) { + Hash.AddQualType(T); + } + void Visit(const Type *T) { ID.AddInteger(T->getTypeClass()); Inherited::Visit(T); @@ -327,6 +342,50 @@ public: VisitType(T); } + void VisitFunctionType(const FunctionType *T) { + AddQualType(T->getReturnType()); + T->getExtInfo().Profile(ID); + Hash.AddBoolean(T->isConst()); + Hash.AddBoolean(T->isVolatile()); + Hash.AddBoolean(T->isRestrict()); + VisitType(T); + } + + void VisitFunctionNoProtoType(const FunctionNoProtoType *T) { + VisitFunctionType(T); + } + + void VisitFunctionProtoType(const FunctionProtoType *T) { + ID.AddInteger(T->getNumParams()); + for (auto ParamType : T->getParamTypes()) + AddQualType(ParamType); + + const auto &epi = T->getExtProtoInfo(); + ID.AddInteger(epi.Variadic); + ID.AddInteger(epi.TypeQuals); + ID.AddInteger(epi.RefQualifier); + ID.AddInteger(epi.ExceptionSpec.Type); + + if (epi.ExceptionSpec.Type == EST_Dynamic) { + for (QualType Ex : epi.ExceptionSpec.Exceptions) + AddQualType(Ex); + } else if (epi.ExceptionSpec.Type == EST_ComputedNoexcept && + epi.ExceptionSpec.NoexceptExpr) { + AddStmt(epi.ExceptionSpec.NoexceptExpr); + } else if (epi.ExceptionSpec.Type == EST_Uninstantiated || + epi.ExceptionSpec.Type == EST_Unevaluated) { + AddDecl(epi.ExceptionSpec.SourceDecl->getCanonicalDecl()); + } + if (epi.ExtParameterInfos) { + for (unsigned i = 0; i != T->getNumParams(); ++i) + ID.AddInteger(epi.ExtParameterInfos[i].getOpaqueValue()); + } + epi.ExtInfo.Profile(ID); + Hash.AddBoolean(epi.HasTrailingReturn); + + VisitFunctionType(T); + } + void VisitTypedefType(const TypedefType *T) { AddDecl(T->getDecl()); Hash.AddQualType(T->getDecl()->getUnderlyingType()); diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 9f50e0541d50..a691f7abd1a5 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9239,6 +9239,11 @@ void ASTReader::diagnoseOdrViolations() { MethodVolatile, MethodConst, MethodInline, + MethodNumberParameters, + MethodParameterType, + MethodParameterName, + MethodParameterSingleDefaultArgument, + MethodParameterDifferentDefaultArguments, }; // These lambdas have the common portions of the ODR diagnostics. This @@ -9563,6 +9568,84 @@ void ASTReader::diagnoseOdrViolations() { break; } + const unsigned FirstNumParameters = FirstMethod->param_size(); + const unsigned SecondNumParameters = SecondMethod->param_size(); + if (FirstNumParameters != SecondNumParameters) { + ODRDiagError(FirstMethod->getLocation(), + FirstMethod->getSourceRange(), MethodNumberParameters) + << FirstName << FirstNumParameters; + ODRDiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), MethodNumberParameters) + << SecondName << SecondNumParameters; + Diagnosed = true; + break; + } + + // Need this status boolean to know when break out of the switch. + bool ParameterMismatch = false; + for (unsigned I = 0; I < FirstNumParameters; ++I) { + const ParmVarDecl *FirstParam = FirstMethod->getParamDecl(I); + const ParmVarDecl *SecondParam = SecondMethod->getParamDecl(I); + if (FirstParam->getType() != SecondParam->getType()) { + ODRDiagError(FirstMethod->getLocation(), + FirstMethod->getSourceRange(), MethodParameterType) + << FirstName << (I + 1) << FirstParam->getType(); + ODRDiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), MethodParameterType) + << SecondName << (I + 1) << SecondParam->getType(); + ParameterMismatch = true; + break; + } + + DeclarationName FirstParamName = FirstParam->getDeclName(); + DeclarationName SecondParamName = SecondParam->getDeclName(); + if (FirstParamName != SecondParamName) { + ODRDiagError(FirstMethod->getLocation(), + FirstMethod->getSourceRange(), MethodParameterName) + << FirstName << (I + 1) << FirstParamName; + ODRDiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), MethodParameterName) + << SecondName << (I + 1) << SecondParamName; + ParameterMismatch = true; + break; + } + + const Expr* FirstDefaultArg = FirstParam->getDefaultArg(); + const Expr* SecondDefaultArg = SecondParam->getDefaultArg(); + if ((!FirstDefaultArg && SecondDefaultArg) || + (FirstDefaultArg && !SecondDefaultArg)) { + ODRDiagError(FirstMethod->getLocation(), + FirstMethod->getSourceRange(), + MethodParameterSingleDefaultArgument) + << FirstName << (I + 1) << (FirstDefaultArg != nullptr); + ODRDiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), + MethodParameterSingleDefaultArgument) + << SecondName << (I + 1) << (SecondDefaultArg != nullptr); + ParameterMismatch = true; + break; + } + + if (ComputeODRHash(FirstDefaultArg) != + ComputeODRHash(SecondDefaultArg)) { + ODRDiagError(FirstMethod->getLocation(), + FirstMethod->getSourceRange(), + MethodParameterDifferentDefaultArguments) + << FirstName << (I + 1); + ODRDiagNote(SecondMethod->getLocation(), + SecondMethod->getSourceRange(), + MethodParameterDifferentDefaultArguments) + << SecondName << (I + 1); + ParameterMismatch = true; + break; + } + } + + if (ParameterMismatch) { + Diagnosed = true; + break; + } + break; } } diff --git a/clang/test/Modules/odr_hash.cpp b/clang/test/Modules/odr_hash.cpp index 75436706200c..6069601b1e83 100644 --- a/clang/test/Modules/odr_hash.cpp +++ b/clang/test/Modules/odr_hash.cpp @@ -403,6 +403,79 @@ S8 s8; // expected-note@first.h:* {{but in 'FirstModule' found method 'A' is const}} #endif +#if defined(FIRST) +struct S9 { + void A(int x) {} + void A(int x, int y) {} +}; +#elif defined(SECOND) +struct S9 { + void A(int x, int y) {} + void A(int x) {} +}; +#else +S9 s9; +// expected-error@second.h:* {{'Method::S9' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' that has 2 parameters}} +// expected-note@first.h:* {{but in 'FirstModule' found method 'A' that has 1 parameter}} +#endif + +#if defined(FIRST) +struct S10 { + void A(int x) {} + void A(float x) {} +}; +#elif defined(SECOND) +struct S10 { + void A(float x) {} + void A(int x) {} +}; +#else +S10 s10; +// expected-error@second.h:* {{'Method::S10' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' with 1st parameter of type 'float'}} +// expected-note@first.h:* {{but in 'FirstModule' found method 'A' with 1st parameter of type 'int'}} +#endif + +#if defined(FIRST) +struct S11 { + void A(int x) {} +}; +#elif defined(SECOND) +struct S11 { + void A(int y) {} +}; +#else +S11 s11; +// expected-error@second.h:* {{'Method::S11' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' with 1st parameter named 'y'}} +// expected-note@first.h:* {{but in 'FirstModule' found method 'A' with 1st parameter named 'x'}} +#endif + +#if defined(FIRST) +struct S12 { + void A(int x) {} +}; +#elif defined(SECOND) +struct S12 { + void A(int x = 1) {} +}; +#else +S12 s12; +// expected-error@second.h:* {{'Method::S12' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' with 1st parameter with default argument}} +// expected-note@first.h:* {{but in 'FirstModule' found method 'A' with 1st parameter with no default argument}} +#endif + +#if defined(FIRST) +struct S13 { + void A(int x = 1 + 0) {} +}; +#elif defined(SECOND) +struct S13 { + void A(int x = 1) {} +}; +#else +S13 s13; +// expected-error@second.h:* {{'Method::S13' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' with 1st parameter with default argument}} +// expected-note@first.h:* {{but in 'FirstModule' found method 'A' with 1st parameter with different default argument}} +#endif } // namespace Method // Naive parsing of AST can lead to cycles in processing. Ensure @@ -522,7 +595,6 @@ S3 s3; #endif } // namespace Using - // 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 { @@ -556,6 +628,9 @@ struct S { typedef int typedef_int; using using_int = int; + + void method_one_arg(int x) {} + void method_one_arg_default_argument(int x = 5 + 5) {} }; #elif defined(SECOND) typedef int INT; @@ -587,6 +662,9 @@ struct S { typedef int typedef_int; using using_int = int; + + void method_one_arg(int x) {} + void method_one_arg_default_argument(int x = 5 + 5) {} }; #else S *s; @@ -623,6 +701,9 @@ struct T { typedef int typedef_int; using using_int = int; + void method_one_arg(int x) {} + void method_one_arg_default_argument(int x = 5 + 5) {} + private: }; #elif defined(SECOND) @@ -656,6 +737,9 @@ struct T { typedef int typedef_int; using using_int = int; + void method_one_arg(int x) {} + void method_one_arg_default_argument(int x = 5 + 5) {} + public: }; #else