NVIDIA / cccl

CUDA Core Compute Libraries
https://nvidia.github.io/cccl/
Other
1.22k stars 156 forks source link

[BUG]: To avoid GCC to fail in preprocessing the __concept header files using "-C" option #1927

Closed zkhatami closed 3 months ago

zkhatami commented 3 months ago

Is this a duplicate?

Type of Bug

Compile-time Error

Component

libcu++

Describe the bug

For this test:

#include "cuda/std/atomic"

GCC started failing during preprocessing with the "-C" option after this commit: https://github.com/NVIDIA/cccl/commit/680c01207c8611c011ba6eed4ba30cb0c8ab2d20#diff-97033c0ba44556602cf7d7eeda4fa26ead8d8ba9ee63b55be99ae263194e124a

This bug in GCC is triggered by the new changes in the __concept header files when using the "-C" option. For example, for this definition of the Movable in cuda12.3 we have:

template<class _Tp>
_LIBCUDACXX_CONCEPT_FRAGMENT(
  _Movable_,
  requires()
  (requires(is_object_v<_Tp>),
   requires(move_constructible<_Tp>),
   requires(assignable_from<_Tp&, _Tp>),
   requires(swappable<_Tp>)));

, while with cuda12.4, we have:

_LIBCUDACXX_CONCEPT_FRAGMENT(
  _Movable_,
  requires()(// <---
    requires(is_object_v<_Tp>),
    requires(move_constructible<_Tp>),
    requires(assignable_from<_Tp&, _Tp>),
    requires(swappable<_Tp>)
  ));

Having the comment "//" within the code causes GCC to fail in the preprocessing phase when using "-C" option:

__concepts/movable.h:46:1: error: pasting "_LIBCUDACXX_CONCEPT_FRAGMENT_REQS_SELECT_PROBE_" and "/**/" does not give a valid preprocessing token
   46 | _LIBCUDACXX_CONCEPT_FRAGMENT(
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Here is the smaller reproducer that confirms this bug in GCC preprocessing when using "-C" option:

#define CONCAT(A, B) A ## B
/* comment */
int CONCAT(ma, /**/ in)() { }

$ g++ -E -C ec.cpp > /dev/null
ec.cpp:3:12: error: pasting "ma" and "/**/" does not give a valid preprocessing token
    3 | int CONCAT(ma, /**/ in)() { }
      |            ^~
ec.cpp:1:22: note: in definition of macro ‘CONCAT’
    1 | #define CONCAT(A, B) A ## B
      |                      

In the commit that CCCL has made, there is no explanation why the comment was added to those __concept header files. If the comment gets eliminated, then GCC would compile this test fine.

How to Reproduce

g++ -E -C ec.cpp

Expected behavior

To not failing in the preprocessing phase.

Reproduction link

No response

Operating System

No response

nvidia-smi output

No response

NVCC version

No response

bernhardmgruber commented 3 months ago

Thank you for reporting this! I assume @miscco added the comments for guiding clang-format into a more pleasant formatting. Maybe replacing the // by a simple /**/ would work.

dkolsen-pgi commented 3 months ago

Replacing // with /**/ won't help. GCC chokes when a comment of any form is in the middle of a token concatenation when comments are preserved by the preprocessor (the -C option).

miscco commented 3 months ago

Those comments were used to force clang-format into submission as it completely mangled these pieces of code.

With our current version this does not happen anymore, so we can drop them alltogether