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

[rnk]

clang/lib/Sema/SemaDeclCXX.cpp

5887–5915

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

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

Thanks, that makes it a bit nicer.

[hans]

Addressing comments.

[thakis]

clang/lib/Sema/SemaDeclCXX.cpp

5906–5907

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

[hans]

clang/lib/Sema/SemaDeclCXX.cpp

5906–5907

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() ||
                  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.
         S.MarkFunctionReferenced(Class->getLocation(), MD);
 
+        if (MD->isExplicitlyDefaulted() &&
+            TSK == TSK_ExplicitInstantiationDefinition) {
+          // MD will get passed to the consumer as part of instantiation.
+        } else {
           // 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.
   if (!S.Context.getTargetInfo().getCXXABI().isMicrosoft())
     return;
 

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"
+}
+
 //===----------------------------------------------------------------------===//
 // 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() {}