Actually delay processing DelayedDllExportClasses until the outermost class is finished (PR40006)

[hans]

Summary

This was already the intention of DelayedDllExportClasses, but this test case would break it:

template<typename> struct Tmpl {};
struct Outer {
    struct Inner {
        __declspec(dllexport) Inner() = default;
        unsigned int x = 0;
    };
    Tmpl<Inner> y;
};

ActOnFinishCXXNonNestedClass() would get called when the instantiation of Templ<Inner> is finished, even though the compiler is still not finished with Outer, causing the compile fail.

This hooks into Sema::{Push,Pop}ParsingClass() to avoid calling ActOnFinishCXXNonNestedClass() for template instantiations while a class is being parsed.

This was the cleanest solution I could come up with :-)

[rnk]

It occurs to me that this will cause some strange ordering in some cases. Consider:

namespace pr40006 {
// Delay emitting the method also past the instantiation of Tmpl<Inner>, i.e.
// until the top-level class Outer is completely finished.
template<typename> struct Tmpl {};
struct Outer {
    struct Inner {
        __declspec(dllexport) Inner() = default;
        unsigned int x = 0;
    };
    // not instantiated
};

struct Other {
    Tmpl<Outer::Inner> y;
};
// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.InClassInits::pr40006::Outer::Inner"* @"??0Inner@Outer@pr40006@InClassInits@@QAE@XZ"
}

When we'd process y before, we would immediately emit delayed dllexported things. Now we will wait until Other is done parsing, which is fine, but not as eager as we could be. That's fine, but it's a slight behavior change.

clang/lib/Sema/SemaTemplateInstantiate.cpp

2232

This function doesn't use its argument. Can you remove it so that is clear that we don't need to track this somewhere else?

[rnk]

I'll add, looks good with suggested fix.

[hans]

Thanks for the review!

clang/lib/Sema/SemaTemplateInstantiate.cpp

2232

Done.

clang/include/clang/Sema/Sema.h

public:
 
   ParsingDeclState PushParsingDeclaration(sema::DelayedDiagnosticPool &pool) {
     return DelayedDiagnostics.push(pool);
   }
   void PopParsingDeclaration(ParsingDeclState state, Decl *decl);
 
   typedef ProcessingContextState ParsingClassState;
   ParsingClassState PushParsingClass() {
+    ParsingClassDepth++;
     return DelayedDiagnostics.pushUndelayed();
   }
   void PopParsingClass(ParsingClassState state) {
+    ParsingClassDepth--;
     DelayedDiagnostics.popUndelayed(state);
   }
 
   void redelayDiagnostics(sema::DelayedDiagnosticPool &pool);
 
   void DiagnoseAvailabilityOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
                                   const ObjCInterfaceDecl *UnknownObjCClass,
                                   bool ObjCPropertyAccess,

   /// conditions that are needed for the attribute to have an effect.
   void checkIllFormedTrivialABIStruct(CXXRecordDecl &RD);
 
   void ActOnFinishCXXMemberSpecification(Scope *S, SourceLocation RLoc,
                                          Decl *TagDecl, SourceLocation LBrac,
                                          SourceLocation RBrac,
                                          const ParsedAttributesView &AttrList);
   void ActOnFinishCXXMemberDecls();
-  void ActOnFinishCXXNonNestedClass(Decl *D);
+  void ActOnFinishCXXNonNestedClass();
 
   void ActOnReenterCXXMethodParameter(Scope *S, ParmVarDecl *Param);
   unsigned ActOnReenterTemplateScope(Scope *S, Decl *Template);
   void ActOnStartDelayedMemberDeclarations(Scope *S, Decl *Record);
   void ActOnStartDelayedCXXMethodDeclaration(Scope *S, Decl *Method);
   void ActOnDelayedCXXMethodParameter(Scope *S, Decl *Param);
   void ActOnFinishDelayedMemberDeclarations(Scope *S, Decl *Record);
   void ActOnFinishDelayedCXXMethodDeclaration(Scope *S, Decl *Method);
public:
   }
 
   // Emitting members of dllexported classes is delayed until the class
   // (including field initializers) is fully parsed.
   SmallVector<CXXRecordDecl*, 4> DelayedDllExportClasses;
   SmallVector<CXXMethodDecl*, 4> DelayedDllExportMemberFunctions;
 
 private:
+  int ParsingClassDepth = 0;
+
   class SavePendingParsedClassStateRAII {
   public:
     SavePendingParsedClassStateRAII(Sema &S) : S(S) { swapSavedState(); }
 
     ~SavePendingParsedClassStateRAII() {
       assert(S.DelayedOverridingExceptionSpecChecks.empty() &&
              "there shouldn't be any pending delayed exception spec checks");
       assert(S.DelayedEquivalentExceptionSpecChecks.empty() &&
              "there shouldn't be any pending delayed exception spec checks");
-      assert(S.DelayedDllExportClasses.empty() &&
-             "there shouldn't be any pending delayed DLL export classes");
       swapSavedState();
     }
 
   private:
     Sema &S;
     decltype(DelayedOverridingExceptionSpecChecks)
         SavedOverridingExceptionSpecChecks;
     decltype(DelayedEquivalentExceptionSpecChecks)
         SavedEquivalentExceptionSpecChecks;
-    decltype(DelayedDllExportClasses) SavedDllExportClasses;
 
     void swapSavedState() {
       SavedOverridingExceptionSpecChecks.swap(
           S.DelayedOverridingExceptionSpecChecks);
       SavedEquivalentExceptionSpecChecks.swap(
           S.DelayedEquivalentExceptionSpecChecks);
-      SavedDllExportClasses.swap(S.DelayedDllExportClasses);
     }
   };
 
   /// Helper class that collects misaligned member designations and
   /// their location info for delayed diagnostics.
   struct MisalignedMember {
     Expr *E;
     RecordDecl *RD;

clang/lib/Parse/ParseDeclCXX.cpp

if (TagDecl && NonNestedClass) {
     Actions.ActOnFinishCXXMemberDecls();
 
     ParseLexedMemberInitializers(getCurrentClass());
     ParseLexedMethodDefs(getCurrentClass());
     PrevTokLocation = SavedPrevTokLocation;
 
     // We've finished parsing everything, including default argument
     // initializers.
-    Actions.ActOnFinishCXXNonNestedClass(TagDecl);
+    Actions.ActOnFinishCXXNonNestedClass();
   }
 
   if (TagDecl)
     Actions.ActOnTagFinishDefinition(getCurScope(), TagDecl, T.getRange());
 
   // Leave the class scope.
   ParsingDef.Pop();
   ClassScope.Exit();

clang/lib/Sema/SemaDeclCXX.cpp

if (Record->isInvalidDecl()) {
       DelayedOverridingExceptionSpecChecks.clear();
       DelayedEquivalentExceptionSpecChecks.clear();
       return;
     }
     checkForMultipleExportedDefaultConstructors(*this, Record);
   }
 }
 
-void Sema::ActOnFinishCXXNonNestedClass(Decl *D) {
+void Sema::ActOnFinishCXXNonNestedClass() {
   referenceDLLExportedClassMethods();
 
   if (!DelayedDllExportMemberFunctions.empty()) {
     SmallVector<CXXMethodDecl*, 4> WorkList;
     std::swap(DelayedDllExportMemberFunctions, WorkList);
     for (CXXMethodDecl *M : WorkList) {
       DefineImplicitSpecialMember(*this, M, M->getLocation());
 

clang/lib/Sema/SemaTemplateInstantiate.cpp

Sema::InstantiateClass(SourceLocation PointOfInstantiation,
   }
 
   // Finish checking fields.
   ActOnFields(nullptr, Instantiation->getLocation(), Instantiation, Fields,
               SourceLocation(), SourceLocation(), ParsedAttributesView());
   CheckCompletedCXXClass(Instantiation);
 
   // Default arguments are parsed, if not instantiated. We can go instantiate
-  // default arg exprs for default constructors if necessary now.
-  ActOnFinishCXXNonNestedClass(Instantiation);
+  // default arg exprs for default constructors if necessary now. Unless we're
+  // parsing a class, in which case wait until that's finished.
+  if (ParsingClassDepth == 0)
+    ActOnFinishCXXNonNestedClass();
 
   // Instantiate late parsed attributes, and attach them to their decls.
   // See Sema::InstantiateAttrs
   for (LateInstantiatedAttrVec::iterator I = LateAttrs.begin(),
        E = LateAttrs.end(); I != E; ++I) {
     assert(CurrentInstantiationScope == Instantiator.getStartingScope());
     CurrentInstantiationScope = I->Scope;
 

clang/test/CodeGenCXX/dllexport.cpp

 // Trying to define the explicitly defaulted ctor must be delayed until the
 // in-class initializer for x has been processed.
 struct PR40006 {
   __declspec(dllexport) PR40006() = default;
   int x = 42;
 };
 // M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.InClassInits::PR40006"* @"??0PR40006@InClassInits@@QAE@XZ"
 
+namespace pr40006 {
+// Delay emitting the method also past the instantiation of Tmpl<Inner>, i.e.
+// until the top-level class Outer is completely finished.
+template<typename> struct Tmpl {};
+struct Outer {
+    struct Inner {
+        __declspec(dllexport) Inner() = default;
+        unsigned int x = 0;
+    };
+    Tmpl<Inner> y;
+};
+// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.InClassInits::pr40006::Outer::Inner"* @"??0Inner@Outer@pr40006@InClassInits@@QAE@XZ"
+}
+
 // PR42857: Clang would try to emit the non-trivial explicitly defaulted
 // dllexport ctor twice when doing an explicit instantiation definition.
 struct Qux { Qux(); };
 template <typename T> struct PR42857 { __declspec(dllexport) PR42857() = default; Qux q; };
 template struct PR42857<int>;
 // M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.InClassInits::PR42857"* @"??0?$PR42857@H@InClassInits@@QAE@XZ"
 
 }