[PATCH] D90299: [windows-itanium] handle dllimport/export code paths separately and share with PS4

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 23 09:17:50 PST 2020

compnerd added inline comments.

Comment at: clang/include/clang/Basic/TargetInfo.h:1096
+  /// Does this target aim for source level compatibility with
+  /// Microsoft C++ code using dllimport/export attributes?
+  virtual bool shouldDLLImportComdatSymbols() const {
I don't think its source compatibility, but rather semantic compatibility.  `-fdeclspec` gives you the source level compatibility - it can process the source, just may not do the same thing.

Comment at: clang/lib/Sema/SemaDecl.cpp:6513
   bool IsInline = false, IsStaticDataMember = false, IsQualifiedFriend = false;
-  bool IsMicrosoft =
-      S.Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-      S.Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment();
+  bool IsMicrosoft = S.Context.getTargetInfo().shouldDLLImportComdatSymbols();
   if (const auto *VD = dyn_cast<VarDecl>(NewDecl)) {
I think that renaming `IsMicrosoft` is now warranted.

Comment at: clang/lib/Sema/SemaTemplate.cpp:9766
         // attribute to a template with a previous instantiation declaration.
         // MinGW doesn't allow this.
         auto *A = cast<InheritableAttr>(
Please update the comment (I think we can entirely remove it now given the new name).  This probably fits better in the definition of `shouldImportDLLImportComdatSymbols` now as the condition is completely different now.



More information about the llvm-commits mailing list