rnk added inline comments.
================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5887-5915 if (MD->isUserProvided()) { // Instantiate non-default class member functions ... // .. except for certain kinds of template specializations. if (TSK == TSK_ImplicitInstantiation && !ClassAttr->isInherited()) continue; ---------------- 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. ================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5898 // encountered. } else if (!MD->isTrivial() || MD->isExplicitlyDefaulted() || MD->isCopyAssignmentOperator() || ---------------- 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. ================ Comment at: clang/test/CodeGenCXX/dllexport.cpp:929 +template <typename> struct T { + T() = default; + X x; ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90849/new/ https://reviews.llvm.org/D90849 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits