Skip to content
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

[clang] Forbidden implicit conversion of constraint expression to bool #54524

Closed
pjgeorg opened this issue Mar 24, 2022 · 3 comments
Closed

[clang] Forbidden implicit conversion of constraint expression to bool #54524

pjgeorg opened this issue Mar 24, 2022 · 3 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts

Comments

@pjgeorg
Copy link

pjgeorg commented Mar 24, 2022

clang (any version) currently accepts any expression that can be implicitly converted to bool in require-clause parenthesized expressions. I first thought this is correct and gcc is wrong in not accepting any constraint expression which does not have type bool.

Searching the C++ standard [temp.constr.atomic]. The example given in 13.5.2.3.3 does indeed compile using clang although the standard clearly states that it is an error. gcc correctly reports the error "constraint expression does not have type 'bool'".

Link to example from C++ Standard in Compiler Explorer: https://godbolt.org/z/5zh6dcT8v

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Mar 24, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 24, 2022

@llvm/issue-subscribers-clang-frontend

@erichkeane
Copy link
Collaborator

This is happening because SemaConcept.cpp ~336 (in calculateConstraintSatisfaction) does a contextual conversion to bool.

The comment says:

         // Substitution might have stripped off a contextual conversion to
         // bool if this is the operand of an '&&' or '||'. For example, we
         // might lose an lvalue-to-rvalue conversion here. If so, put it back
         // before we try to evaluate.

Removing that block DOES result in the error message being correct (error: atomic constraint must be of type 'bool' (found 'S')), but it also causes a bunch of asserts in situations where we assume the result is an 'integral type' (for the purposes of constant evaluation), so it seems we're at least missing some other check somewhere.

@erichkeane
Copy link
Collaborator

Patch here: https://reviews.llvm.org/D141954

erichkeane pushed a commit that referenced this issue Jan 19, 2023
As reported in #54524, and
later in #60038, we were not
properly implmenting temp.constr.atomic P3. This patch stops implicitly
converting constraints to bool, and ensures the Rvalue conversion takes
place as needed.

Differential Revision: https://reviews.llvm.org/D141954
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Jan 20, 2023
As reported in llvm/llvm-project#54524, and
later in llvm/llvm-project#60038, we were not
properly implmenting temp.constr.atomic P3. This patch stops implicitly
converting constraints to bool, and ensures the Rvalue conversion takes
place as needed.

Differential Revision: https://reviews.llvm.org/D141954
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Jun 14, 2024
As reported in llvm/llvm-project#54524, and
later in llvm/llvm-project#60038, we were not
properly implmenting temp.constr.atomic P3. This patch stops implicitly
converting constraints to bool, and ensures the Rvalue conversion takes
place as needed.

Differential Revision: https://reviews.llvm.org/D141954
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" concepts C++20 concepts
Projects
Status: Done
Development

No branches or pull requests

5 participants