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

[Saleem Abdulrasool via Phabricator via llvm-commits]

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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90299/new/

https://reviews.llvm.org/D90299