mstorsjo added inline comments.

================
Comment at: lib/CodeGen/CGVTables.cpp:887-888
 
+  if (CGM.getTriple().isWindowsGNUEnvironment() && 
RD->hasAttr<DLLImportAttr>())
+    return true;
+
----------------
majnemer wrote:
> Maybe a comment like "VTables of classes declared as dllimport are always 
> considered to be external." ?
Ok, I can add that. I wasn't quite sure what to say, since I only ended up here 
in trying to get ShouldUseExternalRTTIDescriptor in ItaniumCXXABI to return 
false. Alternatively I could just add an "if (GNU && DLLImport) return false;" 
in that function instead, if that'd cause less collateral changes (like the 
vtable in dllimport.cpp that changes from available_externally to external).


================
Comment at: lib/CodeGen/CGVTables.cpp:896-900
   // Otherwise, if the class is an instantiated template, the
   // vtable must be defined here.
   if (TSK == TSK_ImplicitInstantiation ||
       TSK == TSK_ExplicitInstantiationDefinition)
     return false;
----------------
majnemer wrote:
> It would be good to have tests for what would have happened if these paths 
> got hit.
Ok - can you hint on what kind of constructs and setups are needed to get here?


https://reviews.llvm.org/D42641



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to