[dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

[rnk]

clang/lib/Sema/SemaDeclCXX.cpp

5887–5919

I wonder if this code block could be simplified by calculating two booleans:

• ShouldReference: If true, call S.MarkFunctionReferenced
• ShouldHandleNow: if true, call HandleTopLevelDecl

We want to mark the vast majority of C++ method decls referenced. The conditions I see here are:

• user provided
• non-trivial
• trivial explicitly defaulted
• trivial copy assign & move assign
• an exception for non-inherited attributes on implicit specializations (?)
• an exception for defaulted explicit instantiations

I tried to come up with a simpler description of those conditions, and couldn't come up with one.

Nevermind. I don't think my suggestion will make the code any simpler, but I'll leave this as a comment for your consideration.

5898–5907

Part of me wants to handle explicitly defaulted things separately from implicit special members, so we don't have to check for explicitly defaulted-ness twice.

clang/test/CodeGenCXX/dllexport.cpp

929

Let's also add a test case where the constructor is explicitly defaulted outside the body of the class. From reading the code, I know we will take the isUserProvided path instead, but I'd like to have it for completeness.

[hans]

clang/lib/Sema/SemaDeclCXX.cpp

5898–5907

Thanks, that makes it a bit nicer.

[hans]

Addressing comments.

[thakis]

clang/lib/Sema/SemaDeclCXX.cpp

5917

should this grow a assert(TSK != TSK_ExplicitInstantiationDefinition) for symmetry?

[hans]

clang/lib/Sema/SemaDeclCXX.cpp

5917

Hmm no, we can still have TSK==TSK_ExplicitInstantiationDefinition here, it's just for explicitly defaulted methods where we don't want to pass it to the consumer.

clang/lib/Sema/SemaDeclCXX.cpp

if (VD && Member->getAttr<DLLExportAttr>() &&
         TSK == TSK_ImplicitInstantiation)
       S.MarkVariableReferenced(VD->getLocation(), VD);
 
     auto *MD = dyn_cast<CXXMethodDecl>(Member);
     if (!MD)
       continue;
 
     if (Member->getAttr<DLLExportAttr>()) {
       if (MD->isUserProvided()) {
         // Instantiate non-default class member functions ...
 
         // .. except for certain kinds of template specializations.
         if (TSK == TSK_ImplicitInstantiation && !ClassAttr->isInherited())
           continue;
 
         S.MarkFunctionReferenced(Class->getLocation(), MD);
 
         // The function will be passed to the consumer when its definition is
         // encountered.
-      } else if (!MD->isTrivial() || MD->isExplicitlyDefaulted() ||
+      } else if (MD->isExplicitlyDefaulted()) {
+        // Synthesize and instantiate explicitly defaulted methods.
+        S.MarkFunctionReferenced(Class->getLocation(), MD);
+
+        if (TSK != TSK_ExplicitInstantiationDefinition) {
+          // Except for explicit instantiation defs, we will not see the
+          // definition again later, so pass it to the consumer now.
+          S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));
+        }
+      } else if (!MD->isTrivial() ||
                  MD->isCopyAssignmentOperator() ||
                  MD->isMoveAssignmentOperator()) {
-        // Synthesize and instantiate non-trivial implicit methods, explicitly
-        // defaulted methods, and the copy and move assignment operators. The
-        // latter are exported even if they are trivial, because the address of
-        // an operator can be taken and should compare equal across libraries.
+        // Synthesize and instantiate non-trivial implicit methods, and the copy
+        // and move assignment operators. The latter are exported even if they
+        // are trivial, because the address of an operator can be taken and
+        // should compare equal across libraries.
         S.MarkFunctionReferenced(Class->getLocation(), MD);
 
         // There is no later point when we will see the definition of this
         // function, so pass it to the consumer now.
         S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));
       }
     }
   }
 }
 
 static void checkForMultipleExportedDefaultConstructors(Sema &S,
                                                         CXXRecordDecl *Class) {
   // Only the MS ABI has default constructor closures, so we don't need to do
   // this semantic checking anywhere else.

clang/test/CodeGenCXX/dllexport.cpp

 struct __declspec(dllexport) ExportedClass { template <typename> void bar() ; };
 template<> void ExportedClass::bar<int>() {}
 // M32-DAG: define dso_local x86_thiscallcc void @"??$bar@H@ExportedClass@pr34849@@QAEXXZ"
 template <typename> struct __declspec(dllexport) ExportedClassTemplate2 { template <typename> void baz(); };
 template<> template<> void ExportedClassTemplate2<int>::baz<int>() {}
 // M32-DAG: define dso_local x86_thiscallcc void @"??$baz@H@?$ExportedClassTemplate2@H@pr34849@@QAEXXZ"
 }
 
+namespace pr47683 {
+struct X { X() {} };
+
+template <typename> struct S {
+  S() = default;
+  X x;
+};
+template struct __declspec(dllexport) S<int>;
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.pr47683::S"* @"??0?$S@H@pr47683@@QAE@XZ"
+
+template <typename> struct T {
+  T() = default;
+  X x;
+};
+extern template struct T<int>;
+template struct __declspec(dllexport) T<int>;
+// Don't assert about multiple codegen for explicitly defaulted method in explicit instantiation def.
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.pr47683::T"* @"??0?$T@H@pr47683@@QAE@XZ"
+
+template <typename> struct U {
+  U();
+  X x;
+};
+template <typename T> U<T>::U() = default;
+extern template struct U<int>;
+template struct __declspec(dllexport) U<int>;
+// Same as T, but with out-of-line ctor.
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.pr47683::U"* @"??0?$U@H@pr47683@@QAE@XZ"
+}
+
 //===----------------------------------------------------------------------===//
 // Classes with template base classes
 //===----------------------------------------------------------------------===//
 
 template <typename T> struct ClassTemplate { void func(); };
 template <typename T> void ClassTemplate<T>::func() {}
 template <typename T> struct __declspec(dllexport) ExportedClassTemplate { void func(); };
 template <typename T> void ExportedClassTemplate<T>::func() {}