Skip to content

Clang does not report invalid arithmetic operations at compile time when doing _Complex math. #84871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tbaederr opened this issue Mar 12, 2024 · 3 comments · Fixed by #90588
Closed
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" constexpr Anything related to constant evaluation

Comments

@tbaederr
Copy link
Contributor

For example:

static_assert(__imag(5.0j / 0) == __builtin_inf());

https://godbolt.org/z/59fqKv4Kc

This works without any diagnostics in Clang, but errors out in gcc.

There are actually a few tests in tests/SemaCXX/complex-folding.cpp that rely on this behavior, although they aren't as obvious as the example above:

// Test that infinities are preserved, don't turn into NaNs, and do form zeros
// when the divisor.
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) * 1.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + __builtin_inf() * 1.0j) * 1.0)) == 1);
static_assert(__builtin_isinf_sign(__real__(1.0 * (__builtin_inf() + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__(1.0 * (1.0 + __builtin_inf() * 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) * (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + 1.0j) * (__builtin_inf() + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) * (__builtin_inf() + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + __builtin_inf() * 1.0j) * (1.0 + 1.0j))) == -1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + __builtin_inf() * 1.0j) * (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + 1.0j) * (1.0 + __builtin_inf() * 1.0j))) == -1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + 1.0j) * (1.0 + __builtin_inf() * 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + __builtin_inf() * 1.0j) * (1.0 + __builtin_inf() * 1.0j))) == -1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + __builtin_inf() * 1.0j) * (__builtin_inf() + __builtin_inf() * 1.0j))) == -1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) / (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__(1.0 + (__builtin_inf() * 1.0j) / (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__((__builtin_inf() + __builtin_inf() * 1.0j) / (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) / 1.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__(1.0 + (__builtin_inf() * 1.0j) / 1.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__((__builtin_inf() + __builtin_inf() * 1.0j) / 1.0)) == 1);
static_assert(((1.0 + 1.0j) / (__builtin_inf() + 1.0j)) == (0.0 + 0.0j));
static_assert(((1.0 + 1.0j) / (1.0 + __builtin_inf() * 1.0j)) == (0.0 + 0.0j));
static_assert(((1.0 + 1.0j) / (__builtin_inf() + __builtin_inf() * 1.0j)) == (0.0 + 0.0j));
static_assert(((1.0 + 1.0j) / __builtin_inf()) == (0.0 + 0.0j));
static_assert(__builtin_isinf_sign(__real__((1.0 + 1.0j) / (0.0 + 0.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + 1.0j) / 0.0)) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) / (0.0 + 0.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + __builtin_inf() * 1.0j) / (0.0 + 0.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__((__builtin_inf() + __builtin_inf() * 1.0j) / (0.0 + 0.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) / 0.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + __builtin_inf() * 1.0j) / 0.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__((__builtin_inf() + __builtin_inf() * 1.0j) / 0.0)) == 1);

In this example:

constexpr _Complex double D1 = {0.0, 0.0};
constexpr double D2 = __real(__builtin_inf() * D1);
static_assert(D2 == 1);

constexpr double D3 = __builtin_inf() * 0.0;

https://godbolt.org/z/dPME9To8d

(which is adapted and simplified from the test above), multiplying inf with 0 produces a nan, which is properly diagnosed when evaluating the initializer for D3, but not at al when evaluating D2. The static_assert proves that D2 is indeed nan.

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Mar 12, 2024
@tbaederr tbaederr added clang:frontend Language frontend issues, e.g. anything involving "Sema" constexpr Anything related to constant evaluation labels Mar 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2024

@llvm/issue-subscribers-clang-frontend

Author: Timm Baeder (tbaederr)

For example: ```c++ static_assert(__imag(5.0j / 0) == __builtin_inf()); ``` https://godbolt.org/z/59fqKv4Kc

This works without any diagnostics in Clang, but errors out in gcc.

There are actually a few tests in tests/SemaCXX/complex-folding.cpp that rely on this behavior, although they aren't as obvious as the example above:

// Test that infinities are preserved, don't turn into NaNs, and do form zeros
// when the divisor.
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) * 1.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + __builtin_inf() * 1.0j) * 1.0)) == 1);
static_assert(__builtin_isinf_sign(__real__(1.0 * (__builtin_inf() + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__(1.0 * (1.0 + __builtin_inf() * 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) * (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + 1.0j) * (__builtin_inf() + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) * (__builtin_inf() + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + __builtin_inf() * 1.0j) * (1.0 + 1.0j))) == -1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + __builtin_inf() * 1.0j) * (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + 1.0j) * (1.0 + __builtin_inf() * 1.0j))) == -1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + 1.0j) * (1.0 + __builtin_inf() * 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + __builtin_inf() * 1.0j) * (1.0 + __builtin_inf() * 1.0j))) == -1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + __builtin_inf() * 1.0j) * (__builtin_inf() + __builtin_inf() * 1.0j))) == -1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) / (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__(1.0 + (__builtin_inf() * 1.0j) / (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__((__builtin_inf() + __builtin_inf() * 1.0j) / (1.0 + 1.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) / 1.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__(1.0 + (__builtin_inf() * 1.0j) / 1.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__((__builtin_inf() + __builtin_inf() * 1.0j) / 1.0)) == 1);
static_assert(((1.0 + 1.0j) / (__builtin_inf() + 1.0j)) == (0.0 + 0.0j));
static_assert(((1.0 + 1.0j) / (1.0 + __builtin_inf() * 1.0j)) == (0.0 + 0.0j));
static_assert(((1.0 + 1.0j) / (__builtin_inf() + __builtin_inf() * 1.0j)) == (0.0 + 0.0j));
static_assert(((1.0 + 1.0j) / __builtin_inf()) == (0.0 + 0.0j));
static_assert(__builtin_isinf_sign(__real__((1.0 + 1.0j) / (0.0 + 0.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((1.0 + 1.0j) / 0.0)) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) / (0.0 + 0.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + __builtin_inf() * 1.0j) / (0.0 + 0.0j))) == 1);
static_assert(__builtin_isinf_sign(__imag__((__builtin_inf() + __builtin_inf() * 1.0j) / (0.0 + 0.0j))) == 1);
static_assert(__builtin_isinf_sign(__real__((__builtin_inf() + 1.0j) / 0.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__((1.0 + __builtin_inf() * 1.0j) / 0.0)) == 1);
static_assert(__builtin_isinf_sign(__imag__((__builtin_inf() + __builtin_inf() * 1.0j) / 0.0)) == 1);

In this example:

constexpr _Complex double D1 = {0.0, 0.0};
constexpr double D2 = __real(__builtin_inf() * D1);
static_assert(D2 == 1);

constexpr double D3 = __builtin_inf() * 0.0;

https://godbolt.org/z/dPME9To8d

(which is adapted and simplified from the test above), multiplying inf with 0 produces a nan, which is properly diagnosed when evaluating the initializer for D3, but not at al when evaluating D2. The static_assert proves that D2 is indeed nan.

@EugeneZelenko EugeneZelenko removed the clang Clang issues not falling into any other category label Mar 12, 2024
@tbaederr
Copy link
Contributor Author

@AaronBallman Do you maybe have an opinion here? Should the behavior be changed? Or is this something some projects may rely on?

@AaronBallman
Copy link
Collaborator

In C, _Complex is morally acceptable in an arithmetic constant expression; C doesn't have a suffix for complex or imaginary numbers so it's technically not possible to have a complex constant in standard C, but we support suffixes as an extension so you can have a complex constant.

In C++, std::complex has all of its operations marked constexpr, so it stands to reason that _Complex in C++ would be roughly equivalent.

So I think complex types should follow the usual expectations for use within a constant expression in that it catches UB as being non-constant, but otherwise is fine. (Because we have an extension allowing arbitrary constant expressions in static_assert rather than requiring they be integer constant expressions specifically.) So I think the behavior probably should be changed?

CC @jcranmer-intel for additional opinions.

tbaederr added a commit that referenced this issue Jun 8, 2024
)

Use handleFloatFloatBinOp to properly diagnose NaN results and divisions
by zero.

Fixes #84871
nekoshirro pushed a commit to nekoshirro/Alchemist-LLVM that referenced this issue Jun 9, 2024
…m#90588)

Use handleFloatFloatBinOp to properly diagnose NaN results and divisions
by zero.

Fixes llvm#84871

Signed-off-by: Hafidz Muzakky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" constexpr Anything related to constant evaluation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants