Skip to content

Conversation

ndrewh
Copy link
Contributor

@ndrewh ndrewh commented Sep 29, 2025

Currently, sanitizer_array_ref_test requires -std=c++17 or newer, due to is_trivially_copyable_v. This adds -std=c++17 to the list of CFLAGS used for compiling sanitizer_common tests.

@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2025

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Andrew Haberlandt (ndrewh)

Changes

Currently, sanitizer_array_ref_test requires -std=c++17 or newer, due to is_trivially_copyable_v. This is the only test that requires c++17, and thus we use the c++14-available std::is_trivially_copyable instead for now, rather than adding the c++17 cflag.


Full diff: https://github.com/llvm/llvm-project/pull/161252.diff

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/tests/sanitizer_array_ref_test.cpp (+1-1)
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_array_ref_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_array_ref_test.cpp
index 12a83df455d39..acb9f950c9914 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_array_ref_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_array_ref_test.cpp
@@ -125,7 +125,7 @@ TEST(ArrayRefTest, ArrayRef) {
   EXPECT_TRUE(ar2.equals(ar2_ref));
 }
 
-static_assert(std::is_trivially_copyable_v<ArrayRef<int>>,
+static_assert(std::is_trivially_copyable<ArrayRef<int>>::value,
               "trivially copyable");
 
 }  // namespace

@DanBlackwell
Copy link
Contributor

LLVM uses C++17 (https://llvm.org/docs/CodingStandards.html#c-standard-versions), is the current version failing to build?

Currently, sanitizer_array_ref_test requires -std=c++17 or newer,
due to is_trivially_copyable_v. This adds -std=c++17 to the
list of CFLAGS used for compiling sanitizer_common tests.
@ndrewh ndrewh force-pushed the sanitizer_array_ref_test_cpp14 branch from 71c4cf6 to d0f4c61 Compare September 29, 2025 18:34
@ndrewh ndrewh changed the title [sanitizer-common] sanitizer_array_ref_test should not require C++17 [sanitizer-common] sanitizer_common unit tests should pass -std=c++17 Sep 29, 2025
@ndrewh
Copy link
Contributor Author

ndrewh commented Sep 29, 2025

OK, let's add the -std=c++17 flag then. I don't know why we aren't passing the flag already given that CMAKE_CXX_STANDARD is 17 and CMAKE_CXX_STANDARD_REQUIRED is YES (here), but I'm not sure it's worth debugging the cmake over this. Other unit tests in compiler-rt also pass the flag.

@ndrewh
Copy link
Contributor Author

ndrewh commented Sep 29, 2025

I think this is probably just a case of "I accidentally used an ancient compiler", but I dunno

@ndrewh ndrewh closed this Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants