[ASTImporter] Add comprehensive tests for ODR violation handling strategies

[martong]

Summary

In this patch we provide additional and comprehensive tests for the ODR
handling strategies. This is the continuation of
https://reviews.llvm.org/D59692.

[martong]

• Fix copy error

[balazske]

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

2

Change the file name here?

[martong]

• Fix wrong file comment

[balazske]

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

37

Probably for (non-member?) functions there is no possibility for conflicting definition (for same prototype)? This can be the case if the function body is different but this is not checked now. For functions no name conflict happens because of overload, at least if C++ is used. I think the tests here with functions are not needed or only for C language. (Do all these pass?)

129

Is this better?

struct VariableTemplate {
  using DeclTy = VarTemplateDecl;
  static constexpr auto *Prototype = "template <class T> extern T X;";
  static constexpr auto *ConflictingPrototype =
      "template <class T> extern float X;";
  static constexpr auto *Definition =
      R"(
      template <class T> T X;
      template <> int X<int>;
      )";
  static constexpr auto *ConflictingDefinition =
      R"(
      template <class T> T X;
      template <> float X<int>;
      )";
  static constexpr auto *ConflictingProtoDef =
      R"(
      template <class T> float X;
      template <> float X<int>;
      )";
  // There is no matcher for varTemplateDecl so use a work-around.
  BindableMatcher<Decl> getPattern() {
    return namedDecl(hasName("X"), unless(isImplicit()),
                     has(templateTypeParmDecl()));
  }
};

152

FunctionTemplate and FunctionTemplateSpec are missing?

[martong]

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

37

We do check these only with lang C. That is handled with the getLang() function, each test use that language which is provided by the getLang() function.

ConflictingDefinition is needed, because we test cases like this (in ImportConflictingDefAfterProto):

void X(int); // TU1
void X(double a) {}; // TU2

This is handled differently by the distinct strategies. I.e. the conflicting definition is not imported when the strategy is Conservative. When it is Liberal then it is imported.

129

I did not want to get this from our fork, becuase template <> int X<int>; seems to be a specialization of a variable template, and that confuses me.

Also, structural equivalency is completely wrong with variable templates, so they are here only for the sake of completeness, the tests which use them are all disabled.

152

Yes, because FunctionTemplates overload with each other. So they are imported always "liberally".

There is no point to liberally import conflicting FunctionTemplateSpecializations.
The only thing we can do in that case is to omit the conflicting declaration.
And this is true in case of ClassTemplateSpecializations too.

Perhaps we should remove struct ClassTemplateSpec as well from here (?).
Because they are never going to be handled "liberally".

@shafik , what do you think about this?

[balazske]

OK
Probably the ClassTemplateSpec can not be handled in liberal way because the AST data structure for template specializations do not allow multiple instances with same argument values?

[martong]

In D66951#1660886, @balazske wrote:

OK
Probably the ClassTemplateSpec can not be handled in liberal way because the AST data structure for template specializations do not allow multiple instances with same argument values?

Yes exactly, that is the case. The implementation holds the specializations in a set, and the key within the set is a hash calculated from the arguments of the specialization.
All specializations are implemented like this, this includes variabletemplatespecializations too.

[shafik]

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

119

Note this is not well-formed b/c it is not initialized, see godbolt

[shafik]

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

119

But it would be ok combined w/ a specialization:

template <>
constexpr int X<int> = 0;

[shafik]

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

152

What you say about FunctionTemplateSpecializations and ClassTemplateSpecializations seems to make sense, importing them liberally would require more than work in the importer.

[martong]

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

119

Ok, I changed the template to have an init expression:

template <class T>
constexpr T X = 0;

152

I have added FunctionTemplate, FunctionTemplateSpec and VarTemplateSpec.

FunctionTemplate decls overload with each other. Thus, they are imported always "liberally". I have added a test for that.

Class and variable template specializations/instantiatons are always imported conservatively, because the AST holds the specializations in a set, and the key within the set is a hash calculated from the arguments of the specialization.
I have added tests for class template specializations, but put the VarTemplateSpec tests into a FIXME, because structural eq does not handle VarTemplates and their specs yet.

Function template specializations are different. They are all "full"
specializations. Structural equivalency does not check the body of
functions, so we cannot create conflicting function template specializations.
Thus, ODR handling strategies has nothing to do with function template
specializations. I have added this as a comment to the tests.

[martong]

• Initialize the variable template in test
• Clean up tests for fun templates and for specializations

[martong]

• Reorganize ODRStrategy independent behaviour tests.

[martong]

I have reorganized the tests to group the ones which test ODR violation strategy independent behaviour.

[balazske]

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

274

This should be called ImportedF instead of PrevF?

[martong]

• PrevF->ImportedF, remove braces

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

274

Ok, renamed to ImportedF.
Also removed the superfluous braces.

[RKSimon]

@martong This is failing on windows buildbots: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19808

[martong]

Trying to fix in svn commit 372633.

[martong]

2nd attempt to fix the windows build errors: 372646

clang/unittests/AST/ASTImporterODRStrategiesTest.cpp

+//===- unittest/AST/ASTImporterODRStrategiesTest.cpp -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Type-parameterized tests to verify the import behaviour in case of ODR
+// violation.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ASTImporterFixtures.h"
+
+namespace clang {
+namespace ast_matchers {
+
+using internal::BindableMatcher;
+
+// DeclTy: Type of the Decl to check.
+// Prototype: "Prototype" (forward declaration) of the Decl.
+// Definition: A definition for the Prototype.
+// ConflictingPrototype: A prototype with the same name but different
+// declaration.
+// ConflictingDefinition: A different definition for Prototype.
+// ConflictingProtoDef: A definition for ConflictingPrototype.
+// getPattern: Return a matcher that matches any of Prototype, Definition,
+// ConflictingPrototype, ConflictingDefinition, ConflictingProtoDef.
+
+struct Function {
+  using DeclTy = FunctionDecl;
+  static constexpr auto *Prototype = "void X(int);";
+  static constexpr auto *ConflictingPrototype = "void X(double);";
+  static constexpr auto *Definition = "void X(int a) {}";
+  static constexpr auto *ConflictingDefinition = "void X(double a) {}";
+  BindableMatcher<Decl> getPattern() {
+    return functionDecl(hasName("X"), unless(isImplicit()));
+  }
+  Language getLang() { return Lang_C; }
+};
+
+struct Typedef {
+  using DeclTy = TypedefNameDecl;
+  static constexpr auto *Definition = "typedef int X;";
+  static constexpr auto *ConflictingDefinition = "typedef double X;";
+  BindableMatcher<Decl> getPattern() { return typedefNameDecl(hasName("X")); }
+  Language getLang() { return Lang_CXX; }
+};
+
+struct TypedefAlias {
+  using DeclTy = TypedefNameDecl;
+  static constexpr auto *Definition = "using X = int;";
+  static constexpr auto *ConflictingDefinition = "using X = double;";
+  BindableMatcher<Decl> getPattern() { return typedefNameDecl(hasName("X")); }
+  Language getLang() { return Lang_CXX11; }
+};
+
+struct Enum {
+  using DeclTy = EnumDecl;
+  static constexpr auto *Definition = "enum X { a, b };";
+  static constexpr auto *ConflictingDefinition = "enum X { a, b, c };";
+  BindableMatcher<Decl> getPattern() { return enumDecl(hasName("X")); }
+  Language getLang() { return Lang_CXX; }
+};
+
+struct EnumConstant {
+  using DeclTy = EnumConstantDecl;
+  static constexpr auto *Definition = "enum E { X = 0 };";
+  static constexpr auto *ConflictingDefinition = "enum E { X = 1 };";
+  BindableMatcher<Decl> getPattern() { return enumConstantDecl(hasName("X")); }
+  Language getLang() { return Lang_CXX; }
+};
+
+struct Class {
+  using DeclTy = CXXRecordDecl;
+  static constexpr auto *Prototype = "class X;";
+  static constexpr auto *Definition = "class X {};";
+  static constexpr auto *ConflictingDefinition = "class X { int A; };";
+  BindableMatcher<Decl> getPattern() {
+    return cxxRecordDecl(hasName("X"), unless(isImplicit()));
+  }
+  Language getLang() { return Lang_CXX; }
+};
+
+struct Variable {
+  using DeclTy = VarDecl;
+  static constexpr auto *Prototype = "extern int X;";
+  static constexpr auto *ConflictingPrototype = "extern float X;";
+  static constexpr auto *Definition = "int X;";
+  static constexpr auto *ConflictingDefinition = "float X;";
+  BindableMatcher<Decl> getPattern() { return varDecl(hasName("X")); }
+  Language getLang() { return Lang_CXX; }
+};
+
+struct ClassTemplate {
+  using DeclTy = ClassTemplateDecl;
+  static constexpr auto *Prototype = "template <class> class X;";
+  static constexpr auto *ConflictingPrototype = "template <int> class X;";
+  static constexpr auto *Definition = "template <class> class X {};";
+  static constexpr auto *ConflictingDefinition =
+      "template <class> class X { int A; };";
+  static constexpr auto *ConflictingProtoDef = "template <int> class X { };";
+  BindableMatcher<Decl> getPattern() {
+    return classTemplateDecl(hasName("X"), unless(isImplicit()));
+  }
+  Language getLang() { return Lang_CXX; }
+};
+
+struct FunctionTemplate {
+  using DeclTy = FunctionTemplateDecl;
+  static constexpr auto *Definition0 =
+      R"(
+      template <class T>
+      void X(T a) {};
+      )";
+  // This is actually not a conflicting definition, but another primary template.
+  static constexpr auto *Definition1 =
+      R"(
+      template <class T>
+      void X(T* a) {};
+      )";
+  BindableMatcher<Decl> getPattern() {
+    return functionTemplateDecl(hasName("X"), unless(isImplicit()));
+  }
+  static std::string getDef0() { return Definition0; }
+  static std::string getDef1() { return Definition1; }
+  Language getLang() { return Lang_CXX; }
+};
+
+static const internal::VariadicDynCastAllOfMatcher<Decl, VarTemplateDecl>
+    varTemplateDecl;
+
+struct VarTemplate {
+  using DeclTy = VarTemplateDecl;
+  static constexpr auto *Definition =
+      R"(
+      template <class T>
+      constexpr T X = 0;
+      )";
+  static constexpr auto *ConflictingDefinition =
+      R"(
+      template <int>
+      constexpr int X = 0;
+      )";
+  BindableMatcher<Decl> getPattern() { return varTemplateDecl(hasName("X")); }
+  Language getLang() { return Lang_CXX14; }
+};
+
+struct ClassTemplateSpec {
+  using DeclTy = ClassTemplateSpecializationDecl;
+  static constexpr auto *Prototype =
+      R"(
+      template <class T> class X;
+      template <> class X<int>;
+      )";
+  static constexpr auto *Definition =
+      R"(
+      template <class T> class X;
+      template <> class X<int> {};
+      )";
+  static constexpr auto *ConflictingDefinition =
+      R"(
+      template <class T> class X;
+      template <> class X<int> { int A; };
+      )";
+  BindableMatcher<Decl> getPattern() {
+    return classTemplateSpecializationDecl(hasName("X"), unless(isImplicit()));
+  }
+  Language getLang() { return Lang_CXX; }
+};
+
+// Function template specializations are all "full" specializations.
+// Structural equivalency does not check the body of functions, so we cannot
+// create conflicting function template specializations.
+struct FunctionTemplateSpec {
+  using DeclTy = FunctionDecl;
+
+  static constexpr auto *Definition0 =
+      R"(
+      template <class T>
+      void X(T a);
+      template <> void X(int a) {};
+      )";
+
+  // This is actually not a conflicting definition, but another full
+  // specialization.
+  // Thus, during the import we would create a new specialization with a
+  // different type argument.
+  static constexpr auto *Definition1 =
+      R"(
+      template <class T>
+      void X(T a);
+      template <> void X(double a) {};
+      )";
+
+  BindableMatcher<Decl> getPattern() {
+    return functionDecl(hasName("X"), isExplicitTemplateSpecialization(),
+                        unless(isImplicit()));
+  }
+  static std::string getDef0() { return Definition0; }
+  static std::string getDef1() { return Definition1; }
+  Language getLang() { return Lang_CXX; }
+};
+
+static const internal::VariadicDynCastAllOfMatcher<
+    Decl, VarTemplateSpecializationDecl>
+    varTemplateSpecializationDecl;
+
+struct VarTemplateSpec {
+  using DeclTy = VarTemplateSpecializationDecl;
+  static constexpr auto *Definition =
+      R"(
+      template <class T> T X = 0;
+      template <> int X<int> = 0;
+      )";
+  static constexpr auto *ConflictingDefinition =
+      R"(
+      template <class T> T X = 0;
+      template <> float X<int> = 1.0;
+      )";
+  BindableMatcher<Decl> getPattern() {
+    return varTemplateSpecializationDecl(hasName("X"), unless(isImplicit()));
+  }
+  Language getLang() { return Lang_CXX14; }
+};
+
+template <typename TypeParam, ASTImporter::ODRHandlingType ODRHandlingParam>
+struct ODRViolation : ASTImporterOptionSpecificTestBase {
+
+  using DeclTy = typename TypeParam::DeclTy;
+
+  ODRViolation() { ODRHandling = ODRHandlingParam; }
+
+  static std::string getPrototype() { return TypeParam::Prototype; }
+  static std::string getConflictingPrototype() {
+    return TypeParam::ConflictingPrototype;
+  }
+  static std::string getDefinition() { return TypeParam::Definition; }
+  static std::string getConflictingDefinition() {
+    return TypeParam::ConflictingDefinition;
+  }
+  static std::string getConflictingProtoDef() {
+    return TypeParam::ConflictingProtoDef;
+  }
+  static BindableMatcher<Decl> getPattern() { return TypeParam().getPattern(); }
+  static Language getLang() { return TypeParam().getLang(); }
+
+  template <std::string (*ToTUContent)(), std::string (*FromTUContent)(),
+            void (*ResultChecker)(llvm::Expected<Decl *> &, Decl *, Decl *)>
+  void TypedTest_ImportAfter() {
+    Decl *ToTU = getToTuDecl(ToTUContent(), getLang());
+    auto *ToD = FirstDeclMatcher<DeclTy>().match(ToTU, getPattern());
+
+    Decl *FromTU = getTuDecl(FromTUContent(), getLang());
+    auto *FromD = FirstDeclMatcher<DeclTy>().match(FromTU, getPattern());
+
+    auto Result = importOrError(FromD, getLang());
+
+    ResultChecker(Result, ToTU, ToD);
+  }
+
+  // Check that a Decl has been successfully imported into a standalone redecl
+  // chain.
+  static void CheckImportedAsNew(llvm::Expected<Decl *> &Result, Decl *ToTU,
+                                 Decl *ToD) {
+    ASSERT_TRUE(isSuccess(Result));
+    Decl *ImportedD = *Result;
+    ASSERT_TRUE(ImportedD);
+    EXPECT_NE(ImportedD, ToD);
+    EXPECT_EQ(DeclCounter<DeclTy>().match(ToTU, getPattern()), 2u);
+
+    // There may be a hidden fwd spec decl before a function spec decl.
+    if (auto *PrevF = dyn_cast<FunctionDecl>(ImportedD)) {
+      if (PrevF->getTemplatedKind() ==
+          FunctionDecl::TK_FunctionTemplateSpecialization) {
+        return;
+      }
+    }
+    EXPECT_FALSE(ImportedD->getPreviousDecl());
+  }
+
+  // Check that a Decl was not imported because of NameConflict.
+  static void CheckImportNameConflict(llvm::Expected<Decl *> &Result,
+                                      Decl *ToTU, Decl *ToD) {
+    EXPECT_TRUE(isImportError(Result, ImportError::NameConflict));
+    EXPECT_EQ(DeclCounter<DeclTy>().match(ToTU, getPattern()), 1u);
+  }
+
+  // Check that a Decl was not imported because lookup found the same decl.
+  static void CheckImportFoundExisting(llvm::Expected<Decl *> &Result,
+                                      Decl *ToTU, Decl *ToD) {
+    ASSERT_TRUE(isSuccess(Result));
+    EXPECT_EQ(DeclCounter<DeclTy>().match(ToTU, getPattern()), 1u);
+  }
+
+  void TypedTest_ImportConflictingDefAfterDef() {
+    TypedTest_ImportAfter<getDefinition, getConflictingDefinition,
+                          CheckImportedAsNew>();
+  }
+  void TypedTest_ImportConflictingProtoAfterProto() {
+    TypedTest_ImportAfter<getPrototype, getConflictingPrototype,
+                          CheckImportedAsNew>();
+  }
+  void TypedTest_ImportConflictingProtoAfterDef() {
+    TypedTest_ImportAfter<getDefinition, getConflictingPrototype,
+                          CheckImportedAsNew>();
+  }
+  void TypedTest_ImportConflictingDefAfterProto() {
+    TypedTest_ImportAfter<getConflictingPrototype, getDefinition,
+                          CheckImportedAsNew>();
+  }
+  void TypedTest_ImportConflictingProtoDefAfterProto() {
+    TypedTest_ImportAfter<getPrototype, getConflictingProtoDef,
+                          CheckImportedAsNew>();
+  }
+  void TypedTest_ImportConflictingProtoAfterProtoDef() {
+    TypedTest_ImportAfter<getConflictingProtoDef, getPrototype,
+                          CheckImportedAsNew>();
+  }
+  void TypedTest_ImportConflictingProtoDefAfterDef() {
+    TypedTest_ImportAfter<getDefinition, getConflictingProtoDef,
+                          CheckImportedAsNew>();
+  }
+  void TypedTest_ImportConflictingDefAfterProtoDef() {
+    TypedTest_ImportAfter<getConflictingProtoDef, getDefinition,
+                          CheckImportedAsNew>();
+  }
+
+  void TypedTest_DontImportConflictingProtoAfterProto() {
+    TypedTest_ImportAfter<getPrototype, getConflictingPrototype,
+                          CheckImportNameConflict>();
+  }
+  void TypedTest_DontImportConflictingDefAfterDef() {
+    TypedTest_ImportAfter<getDefinition, getConflictingDefinition,
+                          CheckImportNameConflict>();
+  }
+  void TypedTest_DontImportConflictingProtoAfterDef() {
+    TypedTest_ImportAfter<getDefinition, getConflictingPrototype,
+                          CheckImportNameConflict>();
+  }
+  void TypedTest_DontImportConflictingDefAfterProto() {
+    TypedTest_ImportAfter<getConflictingPrototype, getDefinition,
+                          CheckImportNameConflict>();
+  }
+  void TypedTest_DontImportConflictingProtoDefAfterProto() {
+    TypedTest_ImportAfter<getPrototype, getConflictingProtoDef,
+                          CheckImportNameConflict>();
+  }
+  void TypedTest_DontImportConflictingProtoAfterProtoDef() {
+    TypedTest_ImportAfter<getConflictingProtoDef, getPrototype,
+                          CheckImportNameConflict>();
+  }
+  void TypedTest_DontImportConflictingProtoDefAfterDef() {
+    TypedTest_ImportAfter<getDefinition, getConflictingProtoDef,
+                          CheckImportNameConflict>();
+  }
+  void TypedTest_DontImportConflictingDefAfterProtoDef() {
+    TypedTest_ImportAfter<getConflictingProtoDef, getDefinition,
+                          CheckImportNameConflict>();
+  }
+
+  // Used for function templates and function template specializations.
+  void TypedTest_ImportDifferentDefAfterDef() {
+    TypedTest_ImportAfter<TypeParam::getDef0, TypeParam::getDef1,
+                          CheckImportedAsNew>();
+  }
+  void TypedTest_DontImportSameDefAfterDef() {
+    TypedTest_ImportAfter<TypeParam::getDef0, TypeParam::getDef0,
+                          CheckImportFoundExisting>();
+  }
+};
+
+// ==============================
+// Define the parametrized tests.
+// ==============================
+
+#define ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(                           \
+    TypeParam, ODRHandlingParam, NamePrefix, TestCase)                         \
+  using TypeParam##ODRHandlingParam =                                          \
+      ODRViolation<TypeParam, ASTImporter::ODRHandlingType::ODRHandlingParam>; \
+  TEST_P(TypeParam##ODRHandlingParam, NamePrefix##TestCase) {                  \
+    TypedTest_##TestCase();                                                    \
+  }
+
+// clang-format off
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Function, Liberal, ,
+    ImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Typedef, Liberal, ,
+    ImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    TypedefAlias, Liberal, ,
+    ImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Enum, Liberal, ,
+    ImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    EnumConstant, Liberal, ,
+    ImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Class, Liberal, ,
+    ImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Variable, Liberal, ,
+    ImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Liberal, ,
+    ImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    VarTemplate, Liberal, ,
+    ImportConflictingDefAfterDef)
+// Class and variable template specializations/instantiatons are always
+// imported conservatively, because the AST holds the specializations in a set,
+// and the key within the set is a hash calculated from the arguments of the
+// specialization.
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplateSpec, Liberal, ,
+    DontImportConflictingDefAfterDef) // Don't import !!!
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    VarTemplateSpec, Liberal, ,
+    DontImportConflictingDefAfterDef) // Don't import !!!
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Function, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Typedef, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    TypedefAlias, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Enum, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    EnumConstant, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Class, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Variable, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    VarTemplate, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplateSpec, Conservative, ,
+    DontImportConflictingDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    VarTemplateSpec, Conservative, ,
+    DontImportConflictingDefAfterDef)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Function, Liberal, ,
+    ImportConflictingProtoAfterProto)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Variable, Liberal, ,
+    ImportConflictingProtoAfterProto)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Liberal, ,
+    ImportConflictingProtoAfterProto)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Function, Conservative, ,
+    DontImportConflictingProtoAfterProto)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Variable, Conservative, ,
+    DontImportConflictingProtoAfterProto)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Conservative, ,
+    DontImportConflictingProtoAfterProto)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Variable, Liberal, ,
+    ImportConflictingProtoAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Liberal, ,
+    ImportConflictingProtoAfterDef)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Variable, Conservative, ,
+    DontImportConflictingProtoAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Conservative, ,
+    DontImportConflictingProtoAfterDef)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Function, Liberal, ,
+    ImportConflictingDefAfterProto)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Variable, Liberal, ,
+    ImportConflictingDefAfterProto)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Liberal, ,
+    ImportConflictingDefAfterProto)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Function, Conservative, ,
+    DontImportConflictingDefAfterProto)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    Variable, Conservative, ,
+    DontImportConflictingDefAfterProto)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Conservative, ,
+    DontImportConflictingDefAfterProto)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Liberal, ,
+    ImportConflictingProtoDefAfterProto)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Conservative, ,
+    DontImportConflictingProtoDefAfterProto)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Liberal, ,
+    ImportConflictingProtoAfterProtoDef)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Conservative, ,
+    DontImportConflictingProtoAfterProtoDef)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Liberal, ,
+    ImportConflictingProtoDefAfterDef)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Conservative, ,
+    DontImportConflictingProtoDefAfterDef)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Liberal, ,
+    ImportConflictingDefAfterProtoDef)
+
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    ClassTemplate, Conservative, ,
+    DontImportConflictingDefAfterProtoDef)
+
+// FunctionTemplate decls overload with each other. Thus, they are imported
+// always as a new node, independently from any ODRHandling strategy.
+//
+// Function template specializations are "full" specializations. Structural
+// equivalency does not check the body of functions, so we cannot create
+// conflicting function template specializations. Thus, ODR handling strategies
+// has nothing to do with function template specializations. Fully specialized
+// function templates are imported as new nodes if their template arguments are
+// different.
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    FunctionTemplate, Liberal, ,
+    ImportDifferentDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    FunctionTemplateSpec, Liberal, ,
+    ImportDifferentDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    FunctionTemplate, Conservative, ,
+    ImportDifferentDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    FunctionTemplateSpec, Conservative, ,
+    ImportDifferentDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    FunctionTemplate, Liberal, ,
+    DontImportSameDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    FunctionTemplateSpec, Liberal, ,
+    DontImportSameDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    FunctionTemplate, Conservative, ,
+    DontImportSameDefAfterDef)
+ASTIMPORTER_ODR_INSTANTIATE_TYPED_TEST_CASE(
+    FunctionTemplateSpec, Conservative, ,
+    DontImportSameDefAfterDef)
+
+// ======================
+// Instantiate the tests.
+// ======================
+
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, FunctionConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, TypedefConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, TypedefAliasConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, EnumConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, EnumConstantConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, ClassConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, VariableConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, ClassTemplateConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, FunctionTemplateConservative,
+    DefaultTestValuesForRunOptions, );
+// FIXME: Make VarTemplate tests work.
+//INSTANTIATE_TEST_CASE_P(
+    //ODRViolationTests, VarTemplateConservative,
+    //DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, FunctionTemplateSpecConservative,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, ClassTemplateSpecConservative,
+    DefaultTestValuesForRunOptions, );
+// FIXME: Make VarTemplateSpec tests work.
+//INSTANTIATE_TEST_CASE_P(
+    //ODRViolationTests, VarTemplateSpecConservative,
+    //DefaultTestValuesForRunOptions, );
+
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, FunctionLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, TypedefLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, TypedefAliasLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, EnumLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, EnumConstantLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, ClassLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, VariableLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, ClassTemplateLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, FunctionTemplateLiberal,
+    DefaultTestValuesForRunOptions, );
+// FIXME: Make VarTemplate tests work.
+// INSTANTIATE_TEST_CASE_P(
+//     ODRViolationTests, VarTemplateLiberal,
+//     DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, ClassTemplateSpecLiberal,
+    DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(
+    ODRViolationTests, FunctionTemplateSpecLiberal,
+    DefaultTestValuesForRunOptions, );
+// FIXME: Make VarTemplateSpec tests work.
+//INSTANTIATE_TEST_CASE_P(
+    //ODRViolationTests, VarTemplateSpecLiberal,
+    //DefaultTestValuesForRunOptions, );
+
+// clang-format on
+
+} // end namespace ast_matchers
+} // end namespace clang

clang/unittests/AST/ASTImporterTest.cpp

TEST_P(ASTImporterOptionSpecificTestBase,
 
   ASSERT_TRUE(ImportedX);
   EXPECT_TRUE(Constr1->isDefaulted());
   EXPECT_TRUE(Constr1->isExplicitlyDefaulted());
   EXPECT_TRUE(Constr2->isDeletedAsWritten());
   EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate());
 }
 
-struct ConflictingDeclsWithLiberalStrategy : ASTImporterOptionSpecificTestBase {
-  ConflictingDeclsWithLiberalStrategy() {
-    this->ODRHandling = ASTImporter::ODRHandlingType::Liberal;
-  }
-};
-
-// Check that a Decl has been successfully imported into a standalone redecl
-// chain.
-template <typename DeclTy, typename PatternTy>
-static void CheckImportedAsNew(llvm::Expected<Decl *> &Result, Decl *ToTU,
-                               PatternTy Pattern) {
-  ASSERT_TRUE(isSuccess(Result));
-  Decl *ImportedD = *Result;
-  ASSERT_TRUE(ImportedD);
-  auto *ToD = FirstDeclMatcher<DeclTy>().match(ToTU, Pattern);
-  EXPECT_NE(ImportedD, ToD);
-  EXPECT_FALSE(ImportedD->getPreviousDecl());
-  EXPECT_EQ(DeclCounter<DeclTy>().match(ToTU, Pattern), 2u);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, Typedef) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      typedef int X;
-      )",
-      Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-      R"(
-      typedef double X;
-      )",
-      Lang_CXX11);
-  auto Pattern = typedefNameDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher<TypedefNameDecl>().match(FromTU, Pattern);
-
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<TypedefNameDecl>(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, TypeAlias) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      using X = int;
-      )",
-      Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-      R"(
-      using X = double;
-      )",
-      Lang_CXX11);
-  auto Pattern = typedefNameDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher<TypedefNameDecl>().match(FromTU, Pattern);
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<TypedefNameDecl>(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, EnumDecl) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      enum X { a, b };
-      )",
-      Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-      R"(
-      enum X { a, b, c };
-      )",
-      Lang_CXX11);
-  auto Pattern = enumDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher<EnumDecl>().match(FromTU, Pattern);
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<EnumDecl>(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, EnumConstantDecl) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      enum E { X = 0 };
-      )",
-      Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-      R"(
-      enum E { X = 1 };
-      )",
-      Lang_CXX11);
-  auto Pattern = enumConstantDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher<EnumConstantDecl>().match(FromTU, Pattern);
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<EnumConstantDecl>(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, RecordDecl) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      class X { int a; };
-      )",
-      Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-      R"(
-      class X { int b; };
-      )",
-      Lang_CXX11);
-  auto Pattern = cxxRecordDecl(hasName("X"), unless(isImplicit()));
-  auto *FromX = FirstDeclMatcher<RecordDecl>().match(FromTU, Pattern);
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<RecordDecl>(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, VarDecl) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      int X;
-      )",
-      Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-      R"(
-      double X;
-      )",
-      Lang_CXX11);
-  auto Pattern = varDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher<VarDecl>().match(FromTU, Pattern);
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<VarDecl>(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, FunctionDecl) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      void X(int);
-      )",
-      Lang_C); // C, no overloading!
-  Decl *FromTU = getTuDecl(
-      R"(
-      void X(double);
-      )",
-      Lang_C);
-  auto Pattern = functionDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<FunctionDecl>(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, ClassTemplateDecl) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      template <class>
-      struct X;
-      )",
-      Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-      R"(
-      template <int>
-      struct X;
-      )",
-      Lang_CXX11);
-  auto Pattern = classTemplateDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher<ClassTemplateDecl>().match(FromTU, Pattern);
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<ClassTemplateDecl>(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, DISABLED_VarTemplateDecl) {
-  const internal::VariadicDynCastAllOfMatcher<Decl, VarTemplateDecl>
-      varTemplateDecl;
-  Decl *ToTU = getToTuDecl(
-      R"(
-      template <class T>
-      constexpr T X;
-      )",
-      Lang_CXX14);
-  Decl *FromTU = getTuDecl(
-      R"(
-      template <int>
-      constexpr int X = 0;
-      )",
-      Lang_CXX14);
-  auto Pattern = varTemplateDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher<VarTemplateDecl>().match(
-      FromTU, varTemplateDecl(hasName("X")));
-  Expected<Decl *> Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew<VarTemplateDecl>(Result, ToTU, Pattern);
-}
-
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, SVEBuiltins,
                         ::testing::Values(ArgVector{"-target",
                                                     "aarch64-linux-gnu"}), );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
                         ::testing::Values(ArgVector()), );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain,
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods,
                         DefaultTestValuesForRunOptions, );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables,
                         DefaultTestValuesForRunOptions, );
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest,
                         DefaultTestValuesForRunOptions, );
 
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, ConflictingDeclsWithLiberalStrategy,
-                        DefaultTestValuesForRunOptions, );
-
 } // end namespace ast_matchers
 } // end namespace clang

clang/unittests/AST/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 if (MSVC)
   set_source_files_properties(ASTImporterTest.cpp PROPERTIES COMPILE_FLAGS /bigobj)
 endif()
 
 add_clang_unittest(ASTTests
   ASTContextParentMapTest.cpp
   ASTImporterFixtures.cpp
   ASTImporterTest.cpp
   ASTImporterGenericRedeclTest.cpp
+  ASTImporterODRStrategiesTest.cpp
   ASTImporterVisibilityTest.cpp
   ASTTraverserTest.cpp
   ASTTypeTraitsTest.cpp
   ASTVectorTest.cpp
   CommentLexer.cpp
   CommentParser.cpp
   CommentTextTest.cpp
   DataCollectionTest.cpp