[libc]: Remove -Wglobal-constructors for libc tests

[vinay-deshmukh]

• Relates to: [libc] re-enable warnings for tests #119281
• Removes -Wglobal-constructors as per: [libc]: Remove -Wglobal-constructors for libc tests #131485 (review)

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[vinay-deshmukh]

@nickdesaulniers @lntue

With regards to: #119281

Do you have any suggestions on how to fix the code for this warning?

AFAICT, by marking the test class instances as static, the warning goes away, but doing so prevents any tests from being run (as the PR checks are failing right now) which I'm not sure why it happens yet.

For what it's worth, the registration mechanism is identical to how Google Test does it , so one way could just be removing this warning for these warnings via pragma/NOLINTBEGIN|NOLINTEND

[lntue]

@nickdesaulniers @lntue

With regards to: #119281

Do you have any suggestions on how to fix the code for this warning?

AFAICT, by marking the test class instances as static, the warning goes away, but doing so prevents any tests from being run (as the PR checks are failing right now) which I'm not sure why it happens yet.

For what it's worth, the registration mechanism is identical to how Google Test does it , so one way could just be removing this warning for these warnings via pragma/NOLINTBEGIN|NOLINTEND

Another possibility is to make the test constructors constexpr instead of static, but I'm not sure how invasive that change would be.

[vinay-deshmukh]

As of 0ab2965 :

while trying to constexpr things (and remove virtual destructor to make Test a literal type which can be constexpr-ed)

fails with:

In file included from /home/runner/work/llvm-project/llvm-project/libc/test/UnitTest/LibcTest.cpp:9:
/home/runner/work/llvm-project/llvm-project/libc/test/UnitTest/LibcTest.h:115:7: warning: '__llvm_libc_21_0_0_git::testing::Test' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
  115 | class Test {
      |       ^
/home/runner/work/llvm-project/llvm-project/libc/test/UnitTest/LibcTest.h:130:26: error: constexpr function never produces a constant expression [-Winvalid-constexpr]
  130 |   constexpr static void *addTest(Test *T) {
      |                          ^~~~~~~
/home/runner/work/llvm-project/llvm-project/libc/test/UnitTest/LibcTest.h:131:9: note: read of non-constexpr variable 'End' is not allowed in a constant expression
  131 |     if (End == nullptr) {
      |         ^
/home/runner/work/llvm-project/llvm-project/libc/test/UnitTest/LibcTest.h:240:16: note: declared here
  240 |   static Test *End;
      |                ^
1 warning and 1 error generated.

I think wrapping with pragma may be the only way here short of disabling the warning for the whole test suite.

Update: even disabling the wglobal-constructor warning is proving difficult as of e8be053, because of how macro definitions are expanded

[vinay-deshmukh]

FWIW: the reason 6b558fb didn't work is:

https://stackoverflow.com/a/5804024

 however, it is not actually guaranteed that this constructor is ever executed unless the object is ODR-used, and the order of initialisation is unsequenced with respect to other initialisations (= hard to predict).

One way to ODR-use it is put that static Type instance; in namespace scope, but then the error for Requires a global destructor shows up, at which point, we are back where we started. And I believe the original declaration/definition is at that scope, to ensure ODR use.

I managed to figure out how to use _Pragma correctly, which allows us to embed #pragma to wrap those "global" constructors with "disable warnings for global constructor", so the constructor runs now, and the tests should be running now.

UPdate: Additionally, making the assumption that all the compilers that are used to build libc, support _Pragma and the GCC prefix for arguments

[vinay-deshmukh]

As of 53bacd0, there are no Werror due to the global-constructors warning

[vinay-deshmukh]

@lntue @nickdesaulniers

This PR is ready for review now

[jhuber6]

I don't think there's any utility in enabling this for the tests. The reason we pass -Werror=global-constructors is because libc itself should never use global objects. The tests, however, want to test the things that implement global constructors (like the integration tests). Also, sometimes a global constructor is the easiest way to initialize something for a test. There's no harm here because it's just a test that's not being shipped and any functional crt1.o should handle it.

[vinay-deshmukh]

I don't think there's any utility in enabling this for the tests. The reason we pass -Werror=global-constructors is because libc itself should never use global objects. The tests, however, want to test the things that implement global constructors (like the integration tests). Also, sometimes a global constructor is the easiest way to initialize something for a test. There's no harm here because it's just a test that's not being shipped and any functional crt1.o should handle it.

Understood, in that case, shall I just remove the commented out line from the .cmake to resolve this part of the issue ?

[vinay-deshmukh]

CI failures seem to be unrelated to this change:

/home/runner/work/llvm-project/llvm-project/build/compiler-rt/lib/fuzzer/libcxx_fuzzer_aarch64/build/include/c++/v1/__configuration/compiler.h:37:8: warning: "Libc++ only supports Clang 19 and later" [-W#warnings]
   37 | #      warning "Libc++ only supports Clang 19 and later"
      |        ^

[vinay-deshmukh]

@jhuber6 You might have to merge this one as I don't have merge permissions. Thanks!

(assuming CI failure is unrelated (almost certainly))