takuto.ikuta added a comment. Hans, thank you for review! I addressed all your comment and fixed small behavior.
================ Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:76 + // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@ + __forceinline void InlineOutclassDefFunc(); + ---------------- hans wrote: > Hmm, I would have thought we should export this function. I see. Added `MD->isThisDeclarationADefinition() ` check in `Sema::checkClassLevelDLLAttribute` This will make change like https://chromium-review.googlesource.com/c/v8/v8/+/1186017 unnecessary. ================ Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:122 +template class TemplateExportedClass<B22>; + + ---------------- hans wrote: > We should also have a test with implicit instantiation, and then the inline > function should not be exported when using /Zc:dllexportInlines-. Added test and `MD->isImplicitlyInstantiable()` check for the test. ================ Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:172 + // INLINE-DAG: declare dllimport i32 @"?InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ"(%class.ImportedClass*) #2 + // NOINLINE-NOT: declare{{.*}}"?InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ" + int InClassDefFuncWithStaticVariable() { ---------------- hans wrote: > This check looks wrong. I think there should be a dllimport declaration for > this function? (Or an available_externally definition if we use -O1?) I think it is OK not dllimporting this function as far as static variable is dllimported and definition of this function is emitted. https://reviews.llvm.org/D51340 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits