Skip to content

Requires expression with local parameter of void type #109538

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
Fedr opened this issue Sep 21, 2024 · 7 comments · Fixed by #109831
Closed

Requires expression with local parameter of void type #109538

Fedr opened this issue Sep 21, 2024 · 7 comments · Fixed by #109831
Assignees
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party diverges-from:edg Does the clang frontend diverge from edg compiler diverges-from:gcc Does the clang frontend diverge from gcc on this issue diverges-from:msvc Does the clang frontend diverge from msvc on this issue good first issue https://github.com/llvm/llvm-project/contribute

Comments

@Fedr
Copy link

Fedr commented Sep 21, 2024

This program is accepted by Clang (requires is evaluated as true):

static_assert( requires(void t) {t;} );

but rejected by GCC and MSVC with hard error: https://gcc.godbolt.org/z/W7f3zqbMb

If it were evaluated as false, it would be more understandable.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" diverges-from:gcc Does the clang frontend diverge from gcc on this issue diverges-from:msvc Does the clang frontend diverge from msvc on this issue and removed new issue labels Sep 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2024

@llvm/issue-subscribers-clang-frontend

Author: Fedor Chelnokov (Fedr)

This program is accepted by Clang (`requires` is evaluated as `true`): ``` static_assert( requires(void t) {t;} ); ``` but rejected by GCC and MSVC with hard error: https://gcc.godbolt.org/z/W7f3zqbMb

If it were evaluated as false, it would be more understandable.

@shafik shafik added the diverges-from:edg Does the clang frontend diverge from edg compiler label Sep 23, 2024
@shafik
Copy link
Collaborator

shafik commented Sep 23, 2024

Looks like a clang bug: https://godbolt.org/z/E7a4eEeKs

I feel like we had another bug like this but can't find it.

CC @erichkeane

@shafik shafik added the confirmed Verified by a second party label Sep 23, 2024
@zyn0217
Copy link
Contributor

zyn0217 commented Sep 24, 2024

This seems to be an oversight in Sema::ActOnStartRequiresExpr() and we forgot to ensure the non-void parameter types in a RequiresExpr.

Like what we did in Sema::BuildFunctionType(), we probably should check if any ParmVarDecl in LocalParameters is of void type, and if so, we should diagnose it with diag::err_param_with_void_type.

Further, we should set the RequiresExprBodyDecl to invalid in that event; we also want to set the satisfaction flag to false in RequiresExpr's constructor so that the expression could be evaluated to false.

@zyn0217 zyn0217 added the good first issue https://github.com/llvm/llvm-project/contribute label Sep 24, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2024

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2024

@llvm/issue-subscribers-good-first-issue

Author: Fedor Chelnokov (Fedr)

This program is accepted by Clang (`requires` is evaluated as `true`): ``` static_assert( requires(void t) {t;} ); ``` but rejected by GCC and MSVC with hard error: https://gcc.godbolt.org/z/W7f3zqbMb

If it were evaluated as false, it would be more understandable.

@c8ef
Copy link
Contributor

c8ef commented Sep 24, 2024

I would like to work on this issue. 💪

@erichkeane
Copy link
Collaborator

I would like to work on this issue. 💪

I've assigned it to you, have fun! Make sure you add a few of us (myself and @zyn0217 included!) to the review, and feel free to ask questions.

erichkeane pushed a commit that referenced this issue Oct 1, 2024
…pression. (#109831)

Fixes #109538.

In this patch, we introduce diagnostic for required expression
parameters in the same way as function parameters, fix the issue of
handling void type parameters, and align the behavior with GCC and other
compilers.
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this issue Oct 3, 2024
…pression. (llvm#109831)

Fixes llvm#109538.

In this patch, we introduce diagnostic for required expression
parameters in the same way as function parameters, fix the issue of
handling void type parameters, and align the behavior with GCC and other
compilers.
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" confirmed Verified by a second party diverges-from:edg Does the clang frontend diverge from edg compiler diverges-from:gcc Does the clang frontend diverge from gcc on this issue diverges-from:msvc Does the clang frontend diverge from msvc on this issue good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants