r300001 - Revert r298824 & r298816, recommit r298742 & r298754

Richard Trieu via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 11 15:32:04 PDT 2017


Author: rtrieu
Date: Tue Apr 11 17:32:03 2017
New Revision: 300001

URL: http://llvm.org/viewvc/llvm-project?rev=300001&view=rev
Log:
Revert r298824 & r298816, recommit r298742 & r298754

r299989 fixes the underlying issue by waiting long enough to late parsed
arguments to be processed before doing an calculating the hash.

r298742
[ODRHash] Add error messages for mismatched parameters in methods.

r298754
[ODRHash] Add support for array and decayed types.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
    cfe/trunk/lib/AST/ODRHash.cpp
    cfe/trunk/lib/Serialization/ASTReader.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=300001&r1=300000&r2=300001&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Tue Apr 11 17:32:03 2017
@@ -146,7 +146,12 @@ def err_module_odr_violation_mismatch_de
   "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%select{| decayed from %8}7|"
+  "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_d
   "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%select{| decayed from %6}5|"
+  "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__">,

Modified: cfe/trunk/lib/AST/ODRHash.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=300001&r1=300000&r2=300001&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ODRHash.cpp (original)
+++ cfe/trunk/lib/AST/ODRHash.cpp Tue Apr 11 17:32:03 2017
@@ -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,14 @@ public:
     }
   }
 
+  void AddQualType(QualType T) {
+    Hash.AddQualType(T);
+  }
+
+  void VisitQualifiers(Qualifiers Quals) {
+    ID.AddInteger(Quals.getAsOpaqueValue());
+  }
+
   void Visit(const Type *T) {
     ID.AddInteger(T->getTypeClass());
     Inherited::Visit(T);
@@ -322,11 +341,92 @@ public:
 
   void VisitType(const Type *T) {}
 
+  void VisitAdjustedType(const AdjustedType *T) {
+    AddQualType(T->getOriginalType());
+    AddQualType(T->getAdjustedType());
+    VisitType(T);
+  }
+
+  void VisitDecayedType(const DecayedType *T) {
+    AddQualType(T->getDecayedType());
+    AddQualType(T->getPointeeType());
+    VisitAdjustedType(T);
+  }
+
+  void VisitArrayType(const ArrayType *T) {
+    AddQualType(T->getElementType());
+    ID.AddInteger(T->getSizeModifier());
+    VisitQualifiers(T->getIndexTypeQualifiers());
+    VisitType(T);
+  }
+  void VisitConstantArrayType(const ConstantArrayType *T) {
+    T->getSize().Profile(ID);
+    VisitArrayType(T);
+  }
+
+  void VisitDependentSizedArrayType(const DependentSizedArrayType *T) {
+    AddStmt(T->getSizeExpr());
+    VisitArrayType(T);
+  }
+
+  void VisitIncompleteArrayType(const IncompleteArrayType *T) {
+    VisitArrayType(T);
+  }
+
+  void VisitVariableArrayType(const VariableArrayType *T) {
+    AddStmt(T->getSizeExpr());
+    VisitArrayType(T);
+  }
+
   void VisitBuiltinType(const BuiltinType *T) {
     ID.AddInteger(T->getKind());
     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());

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=300001&r1=300000&r2=300001&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Apr 11 17:32:03 2017
@@ -9282,6 +9282,11 @@ void ASTReader::diagnoseOdrViolations()
         MethodVolatile,
         MethodConst,
         MethodInline,
+        MethodNumberParameters,
+        MethodParameterType,
+        MethodParameterName,
+        MethodParameterSingleDefaultArgument,
+        MethodParameterDifferentDefaultArguments,
       };
 
       // These lambdas have the common portions of the ODR diagnostics.  This
@@ -9605,6 +9610,104 @@ void ASTReader::diagnoseOdrViolations()
           Diagnosed = true;
           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);
+
+          QualType FirstParamType = FirstParam->getType();
+          QualType SecondParamType = SecondParam->getType();
+          if (FirstParamType != SecondParamType) {
+            if (const DecayedType *ParamDecayedType =
+                    FirstParamType->getAs<DecayedType>()) {
+              ODRDiagError(FirstMethod->getLocation(),
+                           FirstMethod->getSourceRange(), MethodParameterType)
+                  << FirstName << (I + 1) << FirstParamType << true
+                  << ParamDecayedType->getOriginalType();
+            } else {
+              ODRDiagError(FirstMethod->getLocation(),
+                           FirstMethod->getSourceRange(), MethodParameterType)
+                  << FirstName << (I + 1) << FirstParamType << false;
+            }
+
+            if (const DecayedType *ParamDecayedType =
+                    SecondParamType->getAs<DecayedType>()) {
+              ODRDiagNote(SecondMethod->getLocation(),
+                          SecondMethod->getSourceRange(), MethodParameterType)
+                  << SecondName << (I + 1) << SecondParamType << true
+                  << ParamDecayedType->getOriginalType();
+            } else {
+              ODRDiagNote(SecondMethod->getLocation(),
+                          SecondMethod->getSourceRange(), MethodParameterType)
+                  << SecondName << (I + 1) << SecondParamType << false;
+            }
+            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;
       }

Modified: cfe/trunk/test/Modules/odr_hash.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=300001&r1=300000&r2=300001&view=diff
==============================================================================
--- cfe/trunk/test/Modules/odr_hash.cpp (original)
+++ cfe/trunk/test/Modules/odr_hash.cpp Tue Apr 11 17:32:03 2017
@@ -275,6 +275,33 @@ S11 s11;
 // expected-note at first.h:* {{but in 'FirstModule' found field 'x' with a different initializer}}
 #endif
 
+#if defined(FIRST)
+struct S12 {
+  unsigned x[5];
+};
+#elif defined(SECOND)
+struct S12 {
+  unsigned x[7];
+};
+#else
+S12 s12;
+// expected-error at first.h:* {{'Field::S12::x' from module 'FirstModule' is not present in definition of 'Field::S12' in module 'SecondModule'}}
+// expected-note at second.h:* {{declaration of 'x' does not match}}
+#endif
+
+#if defined(FIRST)
+struct S13 {
+  unsigned x[7];
+};
+#elif defined(SECOND)
+struct S13 {
+  double x[7];
+};
+#else
+S13 s13;
+// expected-error at first.h:* {{'Field::S13::x' from module 'FirstModule' is not present in definition of 'Field::S13' in module 'SecondModule'}}
+// expected-note at second.h:* {{declaration of 'x' does not match}}
+#endif
 }  // namespace Field
 
 namespace Method {
@@ -403,6 +430,93 @@ S8 s8;
 // expected-note at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at first.h:* {{but in 'FirstModule' found method 'A' with 1st parameter with different default argument}}
+#endif
+
+#if defined(FIRST)
+struct S14 {
+  void A(int x[2]) {}
+};
+#elif defined(SECOND)
+struct S14 {
+  void A(int x[3]) {}
+};
+#else
+S14 s14;
+// expected-error at second.h:* {{'Method::S14' has different definitions in different modules; first difference is definition in module 'SecondModule' found method 'A' with 1st parameter of type 'int *' decayed from 'int [3]'}}
+// expected-note at first.h:* {{but in 'FirstModule' found method 'A' with 1st parameter of type 'int *' decayed from 'int [2]'}}
+#endif
 }  // namespace Method
 
 // Naive parsing of AST can lead to cycles in processing.  Ensure
@@ -522,71 +636,54 @@ 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.
+#define ALL_DECLS                                        \
+public:                                                  \
+private:                                                 \
+protected:                                               \
+  static_assert(1 == 1, "Message");                      \
+  static_assert(2 == 2);                                 \
+                                                         \
+  int x;                                                 \
+  double y;                                              \
+                                                         \
+  INT z;                                                 \
+                                                         \
+  unsigned a : 1;                                        \
+  unsigned b : 2 * 2 + 5 / 2;                            \
+                                                         \
+  mutable int c = sizeof(x + y);                         \
+                                                         \
+  void method() {}                                       \
+  static void static_method() {}                         \
+  virtual void virtual_method() {}                       \
+  virtual void pure_virtual_method() = 0;                \
+  inline void inline_method() {}                         \
+  void volatile_method() volatile {}                     \
+  void const_method() const {}                           \
+                                                         \
+  typedef int typedef_int;                               \
+  using using_int = int;                                 \
+                                                         \
+  void method_one_arg(int x) {}                          \
+  void method_one_arg_default_argument(int x = 5 + 5) {} \
+  void method_decayed_type(int x[5]) {}                  \
+                                                         \
+  int constant_arr[5];                                   \
+                                                         \
+  double last_decl;
+
 namespace AllDecls {
 #if defined(FIRST)
 typedef int INT;
 struct S {
-  public:
-  private:
-  protected:
-
-  static_assert(1 == 1, "Message");
-  static_assert(2 == 2);
-
-  int x;
-  double y;
-
-  INT z;
-
-  unsigned a : 1;
-  unsigned b : 2*2 + 5/2;
-
-  mutable int c = sizeof(x + y);
-
-  void method() {}
-  static void static_method() {}
-  virtual void virtual_method() {}
-  virtual void pure_virtual_method() = 0;
-  inline void inline_method() {}
-  void volatile_method() volatile {}
-  void const_method() const {}
-
-  typedef int typedef_int;
-  using using_int = int;
+  ALL_DECLS
 };
 #elif defined(SECOND)
 typedef int INT;
 struct S {
-  public:
-  private:
-  protected:
-
-  static_assert(1 == 1, "Message");
-  static_assert(2 == 2);
-
-  int x;
-  double y;
-
-  INT z;
-
-  unsigned a : 1;
-  unsigned b : 2 * 2 + 5 / 2;
-
-  mutable int c = sizeof(x + y);
-
-  void method() {}
-  static void static_method() {}
-  virtual void virtual_method() {}
-  virtual void pure_virtual_method() = 0;
-  inline void inline_method() {}
-  void volatile_method() volatile {}
-  void const_method() const {}
-
-  typedef int typedef_int;
-  using using_int = int;
+  ALL_DECLS
 };
 #else
 S *s;
@@ -595,66 +692,14 @@ S *s;
 #if defined(FIRST)
 typedef int INT;
 struct T {
-  public:
-  private:
-  protected:
-
-  static_assert(1 == 1, "Message");
-  static_assert(2 == 2);
-
-  int x;
-  double y;
-
-  INT z;
-
-  unsigned a : 1;
-  unsigned b : 2 * 2 + 5 / 2;
-
-  mutable int c = sizeof(x + y);
-
-  void method() {}
-  static void static_method() {}
-  virtual void virtual_method() {}
-  virtual void pure_virtual_method() = 0;
-  inline void inline_method() {}
-  void volatile_method() volatile {}
-  void const_method() const {}
-
-  typedef int typedef_int;
-  using using_int = int;
+  ALL_DECLS
 
   private:
 };
 #elif defined(SECOND)
 typedef int INT;
 struct T {
-  public:
-  private:
-  protected:
-
-  static_assert(1 == 1, "Message");
-  static_assert(2 == 2);
-
-  int x;
-  double y;
-
-  INT z;
-
-  unsigned a : 1;
-  unsigned b : 2 * 2 + 5 / 2;
-
-  mutable int c = sizeof(x + y);
-
-  void method() {}
-  static void static_method() {}
-  virtual void virtual_method() {}
-  virtual void pure_virtual_method() = 0;
-  inline void inline_method() {}
-  void volatile_method() volatile {}
-  void const_method() const {}
-
-  typedef int typedef_int;
-  using using_int = int;
+  ALL_DECLS
 
   public:
 };
@@ -944,6 +989,22 @@ T t;
 #endif
 }  // namespace StructWithForwardDeclarationNoDefinition
 
+namespace LateParsedDefaultArgument {
+#if defined(FIRST)
+template <typename T>
+struct S {
+  struct R {
+    void foo(T x = 0) {}
+  };
+};
+#elif defined(SECOND)
+#else
+void run() {
+  S<int>::R().foo();
+}
+#endif
+}
+
 // Keep macros contained to one file.
 #ifdef FIRST
 #undef FIRST




More information about the cfe-commits mailing list