Libcudacxx flag cleanup

[alliepiper]

Removes some unused headertest flag variables.

Adds CMAKE_CUDA_FLAGS and CMAKE_CUDA_FLAGS_<config> to lit builds.

Thanks to @davebayer for noticing these flags missing from lit targets.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[alliepiper]

/ok to test

[davebayer]

There is actually a huge problem that I was solving in #4996. The problem that we use assert(...) in lit tests, but CMake in release build type (I believe it's CMake) forces NDEBUG to be defined which means that none of the assertions are actually evaluated.

[alliepiper]

There is actually a huge problem that I was solving in #4996. The problem that we use assert(...) in lit tests, but CMake in release build type (I believe it's CMake) forces NDEBUG to be defined which means that none of the assertions are actually evaluated.

Oof.

AIUI, we could switch these to _CCCL_VERIFY (or _CCCL_ASSERT with the proper flags enabled) and they should still work: https://github.com/alliepiper/cccl/blob/libcudacxx_flags/libcudacxx/include/cuda/std/__cccl/assert.h#L138

@miscco is this correct? Do you have thoughts on this?

(incidentally I also noticed a bug in the docs for the VERIFY macro)

[alliepiper]

There are also a toooon of warnings uncovered by these new flags in the lit tests.

[davebayer]

AIUI, we could switch these to _CCCL_VERIFY (or _CCCL_ASSERT with the proper flags enabled) and they should still work:

We can use something like TEST_ASSERT, or reimplement some of the c2h macros

There are also a toooon of warnings uncovered by these new flags in the lit tests.

That's because there are variables that are defined and only used inside assert(...). Now when assert expands to (void)0, they are unused. I think it will be fine once we undefine NDEBUG

[alliepiper]

Let's switch to a new macro instead of using assert. We shouldn't be using raw asserts to verify correctness in tests for exactly this reason 😄Too fragile.

[miscco]

@miscco is this correct? Do you have thoughts on this?

Its not, we do not use cmake to compile the lit tests.

So asserts are properly live, its just that there are new warnings added

[github-actions]

😬 CI Workflow Results

🟥 Finished in 3h 14m: Pass: 43%/89 | Total: 2d 07h | Max: 3h 02m | Hits: 92%/31102

See results here.

[alliepiper]

its just that there are new warnings added

This does not introduce new warning flags. It only adds -O3 -DNDEBUG to the CI builds.

[miscco]

its just that there are new warnings added

This does not introduce new warning flags. It only adds -O3 -DNDEBUG to the CI builds.

this compiles out all asserts, which adds a bazillion unused variable warnings because the tests do not do anything anymore

[miscco]

We need more discussion here