[clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

[mstorsjo]

Summary

This fixes running tests with a toolchain that defaults to a MinGW
target.

[hans]

lgtm

[thakis]

This broke check-clang on windows: http://45.33.8.238/win/78359/step_7.txt

(The Driver/split-debug.c failure is something else and since fixed, but the other two tests are due to this change.)

Please take a look and revert for now if it takes a while to fix.

[thakis]

Fails with cmake too: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8781510258907322449/+/u/package_clang/stdout?format=raw Reverted in 56bac6c87a2d9046dbac36aae4910b4b90edf643 for now.

[mstorsjo]

In D149997#4331937, @thakis wrote:

This broke check-clang on windows: http://45.33.8.238/win/78359/step_7.txt

(The Driver/split-debug.c failure is something else and since fixed, but the other two tests are due to this change.)

Please take a look and revert for now if it takes a while to fix.

Sorry about this, and thanks for reverting. I’ll have a closer look at the issue later.

[mstorsjo]

So, the reason why this failed, is that when invoked as %clang_cc1 in a MSVC/clang-cl style build, _MSC_VER isn't predefined, while _WIN32 is. When invoked via the Clang driver instead of directly going at -cc1, _MSC_VER does get defined.

@rnk @hans @thakis - who know the intricacies of the MSVC target - do you happen to know why that is? How do I distinguish between MSVC-style behaviour and other cases (in particular, mingw) in a %clang_cc1 test? Currently it uses #idef _WIN32 but that's incorrect for mingw.

[rnk]

In D149997#4337548, @mstorsjo wrote:

So, the reason why this failed, is that when invoked as %clang_cc1 in a MSVC/clang-cl style build, _MSC_VER isn't predefined, while _WIN32 is. When invoked via the Clang driver instead of directly going at -cc1, _MSC_VER does get defined.

@rnk @hans @thakis - who know the intricacies of the MSVC target - do you happen to know why that is? How do I distinguish between MSVC-style behaviour and other cases (in particular, mingw) in a %clang_cc1 test? Currently it uses #idef _WIN32 but that's incorrect for mingw.

I think _MSC_EXTENSIONS will work, but it's kind of gross. _MSC_VER is controlled by -fms-compatibility-verson=, which I think is off by default or unset at the cc1 level:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/OSTargets.cpp#L206
-fms-extensions is on by default at cc1, I think.

This isn't great. It would be nice to have some kind of feature test mechanism for "which C++ ABI am I using, Itanium, one of the variants, or Microsoft".

[mstorsjo]

In D149997#4338304, @rnk wrote:

I think _MSC_EXTENSIONS will work, but it's kind of gross. _MSC_VER is controlled by -fms-compatibility-verson=, which I think is off by default or unset at the cc1 level:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/OSTargets.cpp#L206
-fms-extensions is on by default at cc1, I think.

Ah, I see, thanks!

Since _MSC_VER is the most canonical way of checking for an MSVC-like environment anyway, do you think it'd make sense to define it to a dummy value like 1, if the target is an MSVC triple but no -fms-compatibility-version= is provided?

Using _MSC_EXTENSIONS instead of _MSC_VER in these tests isn't very pretty, but I guess it could be passable still? (I'm not setting out to improve all the world here right now, I'm just trying to get the tests passing in a mingw environment.)

[mstorsjo]

Updated to check for defined(_WIN32) && !defined(__MINGW32__). It's a kinda ugly way of checking for an MSVC-like environment specifically (when the MSVC mode is the exception compared with the rest), but neither _MSC_VER or _MSC_EXTENSIONS are defined by default in a clang -cc1 invocation.

[aaron.ballman]

In D149997#4338897, @mstorsjo wrote:

Updated to check for defined(_WIN32) && !defined(__MINGW32__). It's a kinda ugly way of checking for an MSVC-like environment specifically (when the MSVC mode is the exception compared with the rest), but neither _MSC_VER or _MSC_EXTENSIONS are defined by default in a clang -cc1 invocation.

This seems like forward progress to me, so LG!

clang/test/C/drs/dr1xx.c

void dr118(void) {
    * implementation may complete the type earlier if it has sufficient type
    * information to calculate size or alignment, etc.
    *
    * On Microsoft targets, an enum is always implicit int sized, so the type
    * is sufficiently complete there. On other platforms, it is an incomplete
    * type at this point.
    */
     Val = sizeof(enum E)
-    #ifndef _WIN32
+    #if !defined(_WIN32) || defined(__MINGW32__)
     /* expected-error@-2 {{invalid application of 'sizeof' to an incomplete type 'enum E'}} */
     /* expected-note@-12 {{definition of 'enum E' is not complete until the closing '}'}} */
     #endif
   };
 }
 
 /* WG14 DR119: yes
  * Initialization of multi-dimensional array objects

clang/test/Driver/experimental-library-flag.cpp

 // On some platforms, -stdlib=libc++ is currently ignored, so -lc++experimental is not added.
 // Once -stdlib=libc++ works on those, this XFAIL can be removed.
-// XFAIL: target={{.*-windows.*}}, target={{.*-(ps4|ps5)}}
+// XFAIL: target={{.*-windows-msvc.*}}, target={{.*-(ps4|ps5)}}
 
 // For some reason, this fails with a core dump on AIX. This needs to be investigated.
 // UNSUPPORTED: target={{.*}}-aix{{.*}}
 
 // RUN: %clangxx -fexperimental-library -stdlib=libc++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIBCXX %s
 // RUN: %clangxx -fexperimental-library -stdlib=libstdc++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-LIBSTDCXX %s
 // RUN: %clangxx -fexperimental-library -stdlib=libc++ -nostdlib++ -### %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK-NOSTDLIB %s
 
 // -fexperimental-library must be passed to CC1.
 // CHECK: -fexperimental-library
 
 // Depending on the stdlib in use, we should (or not) pass -lc++experimental.
 // CHECK-LIBCXX: -lc++experimental
 // CHECK-LIBSTDCXX-NOT: -lc++experimental
 // CHECK-NOSTDLIB-NOT: -lc++experimental

clang/test/SemaCXX/attr-trivial-abi.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
 
 void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}}
 
 // Should not crash.
 template <class>
 class __attribute__((trivial_abi)) a { a(a &&); };
-#ifdef _WIN64
-// On Windows, to be trivial-for-calls, an object must be trivially copyable.
+#if defined(_WIN64) && !defined(__MINGW32__)
+// On Windows/MSVC, to be trivial-for-calls, an object must be trivially copyable.
 // (And it is only trivially relocatable, currently, if it is trivial for calls.)
 // In this case, it is suppressed by an explicitly defined move constructor.
-// Similar concerns apply to later tests that have #ifdef _WIN64.
+// Similar concerns apply to later tests that have #if defined(_WIN64) && !defined(__MINGW32__)
 static_assert(!__is_trivially_relocatable(a<int>), "");
 #else
 static_assert(__is_trivially_relocatable(a<int>), "");
 #endif
 
 struct [[clang::trivial_abi]] S0 {
   int a;
 };

 #else
 static_assert(!__is_trivially_relocatable(S18), "");
 #endif
 
 struct __attribute__((trivial_abi)) CopyDeleted {
   CopyDeleted(const CopyDeleted &) = delete;
   CopyDeleted(CopyDeleted &&) = default;
 };
-#ifdef _WIN64
+#if defined(_WIN64) && !defined(__MINGW32__)
 static_assert(!__is_trivially_relocatable(CopyDeleted), "");
 #else
 static_assert(__is_trivially_relocatable(CopyDeleted), "");
 #endif
 
 struct __attribute__((trivial_abi)) MoveDeleted {
   MoveDeleted(const MoveDeleted &) = default;
   MoveDeleted(MoveDeleted &&) = delete;

 #else
 static_assert(!__is_trivially_relocatable(S19), "");
 #endif
 
 // This is fine since the move constructor isn't deleted.
 struct __attribute__((trivial_abi)) S20 {
   int &&a; // a member of rvalue reference type deletes the copy constructor.
 };
-#ifdef _WIN64
+#if defined(_WIN64) && !defined(__MINGW32__)
 static_assert(!__is_trivially_relocatable(S20), "");
 #else
 static_assert(__is_trivially_relocatable(S20), "");
 #endif
 } // namespace deletedCopyMoveConstructor