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

consteval calls in default args #54744

Closed
urnathan opened this issue Apr 4, 2022 · 5 comments
Closed

consteval calls in default args #54744

urnathan opened this issue Apr 4, 2022 · 5 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" consteval C++20 consteval

Comments

@urnathan
Copy link
Contributor

urnathan commented Apr 4, 2022

Consteval calls in default args do not work

consteval int here() { return 5; }
void error(int location = here()) { }
void bob () { error(); }

The problem here is we markused the consteval fn, which causes us to emit an error about taking the address of the consteval fn. I patched up EvaluatedExprMarker, called from Sema::CheckCXXDefaultArgExpr to skip such consteval calls, but we then ICE in code emission, trying to emit a consteval fn body.

https://bugs.llvm.org/show_bug.cgi?id=5810 seems relevant, as that's mentioned in the comment in CheckCXXDefaultArgExpr:
// FIXME: We should really be rebuilding the default argument with new
// bound temporaries; see the comment in PR5810.
// We don't need to do that with block decls, though, because
// blocks in default argument expression can never capture anything.

5810 includes the ominous note:
One could imagine rebuilding the AST nodes with new CXXTemporary pointers, although that's going to require a lot of code :(

Notice that because of __source_location(), which is different on every evaluation, we can't just morph the original defaultArgExpr as is held in the ParamDecl. We have to clone and evaluate at each use, I think.

(this default arg pattern is used in the source_location header to get the location corresponding to the caller of the source_location ctor, not the location of the ctor itself)

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

llvmbot commented Apr 4, 2022

@llvm/issue-subscribers-clang-frontend

@tbaederr
Copy link
Contributor

tbaederr commented Apr 5, 2022

This is a duplicate of #48230 afaics.

@urnathan
Copy link
Contributor Author

urnathan commented Apr 5, 2022

It appears to be the same -- the problem doesn't just effect consteval ctors. The recent addition of builtin_source_location (https://reviews.llvm.org/rGd61487490022) seems to have exposed this issue -- suddenly the (gcc) libstdc++ source_location header bursts into light, and then goes down in flames :(

@ecatmur
Copy link

ecatmur commented Jul 17, 2022

#48230 is fixed by a4f8590, should this be closed?

@usx95
Copy link
Contributor

usx95 commented Aug 20, 2022

This is fixed at head.
https://godbolt.org/z/nvjb7hEoh

@usx95 usx95 closed this as completed Aug 20, 2022
@usx95 usx95 added the consteval C++20 consteval label Aug 20, 2022
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" consteval C++20 consteval
Projects
Status: Done
Development

No branches or pull requests

6 participants