[libcxx] [test] Avoid conflicting definitions of _CRT_SECURE_NO_WARNINGS

[mstorsjo]

Summary

This is defined both by libcxx/utils/libcxx/test/config.py (for any windows target) and msvc_stdlib_force_include.h (when testing specifically the MSVC C++ library).

The command line define (-D_CRT_SECURE_NO_WARNINGS) defines it to the value 1; change the header define to match that.

Keeping both instances, to keep the fix for cases when not building in cases that don't use config.py.

Also remove a comment about whether this can be removed; it can't at least be removed altogether - doing that breaks a number of tests that otherwise succeed.

[ldionne]

Instead, do you think it would make sense to only keep this #define? I'm trying to remove logic in config.py.

[mstorsjo]

In D89588#2335828, @ldionne wrote:

Instead, do you think it would make sense to only keep this #define? I'm trying to remove logic in config.py.

The difference is that this header is only included when testing the MS STL, while the defines in config.py are added also when testing libc++ itself. These defines are used to silence things in the MSVC C standard library, which would be used (presumably by the test support code) even if testing libc++ itself.

I'm rerunning the whole testsuite to see if this particular define can be removed from config.py as well (there's a fixme comment asking whether it can be removed).

For the filesystem stuff, I'd like to add _CRT_NONSTDC_NO_WARNINGS similarly, which avoids needing to do anything at all regarding the functions chdir, fileno and getcwd in the support header.

[mstorsjo]

In D89588#2335881, @mstorsjo wrote:

I'm rerunning the whole testsuite to see if this particular define can be removed from config.py as well (there's a fixme comment asking whether it can be removed).

No, removing the define altogether breaks 45 tests that succeeded before.

[mstorsjo]

Also remove a fixme comment regarding this define - it can't be removed from config.py.

[CaseyCarter]

libcxx/test/support/msvc_stdlib_force_include.h

17

Instead of striking this, could we instead define it conditionally to 1:

#ifndef _CRT_SECURE_NO_WARNINGS
#define _CRT_SECURE_NO_WARNINGS 1
#endif // _CRT_SECURE_NO_WARNINGS

to preserve use cases that don't involve config.py?

[mstorsjo]

libcxx/test/support/msvc_stdlib_force_include.h

17

Sure, that's fine by me as well. And no new ifdef is needed if we just add the 1 to the definition.

[CaseyCarter]

libcxx/test/support/msvc_stdlib_force_include.h

17

Yes, thanks - I forget that redefinition is allowed when the replacement lists are identical.

[mstorsjo]

Kept both instances of the define but adjusted them to match.

[STL_MSFT]

Making the definitions non-conflicting looks good to me, thanks.

[mstorsjo]

@ldionne Any more comments on this?

libcxx/test/support/msvc_stdlib_force_include.h

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef SUPPORT_MSVC_STDLIB_FORCE_INCLUDE_H
 #define SUPPORT_MSVC_STDLIB_FORCE_INCLUDE_H
 
 // This header is force-included when running the libc++ tests against the
 // MSVC standard library.
 
 #ifndef _LIBCXX_IN_DEVCRT
     // Silence warnings about CRT machinery.
-    #define _CRT_SECURE_NO_WARNINGS
+    #define _CRT_SECURE_NO_WARNINGS 1
 
     // Avoid assertion dialogs.
     #define _CRT_SECURE_INVALID_PARAMETER(EXPR) ::abort()
 #endif // _LIBCXX_IN_DEVCRT
 
 #include <crtdbg.h>
 #include <stdlib.h>
 

libcxx/utils/libcxx/test/config.py

def configure_features(self):
             self.cxx.libcxx_gdb = libcxx_gdb
 
     def configure_compile_flags(self):
         self.configure_default_compile_flags()
         # Configure extra flags
         compile_flags_str = self.get_lit_conf('compile_flags', '')
         self.cxx.compile_flags += shlex.split(compile_flags_str)
         if self.target_info.is_windows():
-            # FIXME: Can we remove this?
             self.cxx.compile_flags += ['-D_CRT_SECURE_NO_WARNINGS']
             # Don't warn about using common but nonstandard unprefixed functions
             # like chdir, fileno.
             self.cxx.compile_flags += ['-D_CRT_NONSTDC_NO_WARNINGS']
             # Required so that tests using min/max don't fail on Windows,
             # and so that those tests don't have to be changed to tolerate
             # this insanity.
             self.cxx.compile_flags += ['-DNOMINMAX']