aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:9602-9604
+ return Ctx.Context ==
+ ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed ||
+ Ctx.IsCheckingDefaultArgumentOrInitializer;
----------------
Hmm, it'd be nice to not name this with the same identifier as the `bool`
member on line 1333, that surprised me a little bit when I ran into it below.
================
Comment at: clang/include/clang/Sema/Sema.h:9610
+ "Must be in an expression evaluation context");
+ for (auto &Ctx : llvm::reverse(ExprEvalContexts)) {
+ if (Ctx.Context ==
----------------
================
Comment at: clang/include/clang/Sema/Sema.h:9628
+ if (Ctx.Context == ExpressionEvaluationContext::PotentiallyEvaluated &&
+ Ctx.DelayedDefaultInitializationContext.has_value())
+ return Ctx.DelayedDefaultInitializationContext;
----------------
================
Comment at: clang/include/clang/Sema/Sema.h:9645-9646
+ if (Ctx.Context == ExpressionEvaluationContext::PotentiallyEvaluated &&
+ !Ctx.DelayedDefaultInitializationContext.has_value() &&
+ Res.has_value())
+ break;
----------------
================
Comment at: clang/lib/AST/ExprCXX.cpp:964
+ DeclContext *UsedContext) {
+ size_t Size = totalSizeToAlloc<Expr *>(RewrittenExpr != nullptr ? 1 : 0);
+ auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));
----------------
================
Comment at: clang/lib/AST/ExprCXX.cpp:1019
+
+ size_t Size = totalSizeToAlloc<Expr *>(RewrittenInitExpr != nullptr ? 1 : 0);
+ auto *Mem = Ctx.Allocate(Size, alignof(CXXDefaultArgExpr));
----------------
================
Comment at: clang/lib/Sema/SemaExpr.cpp:5927
+ bool VisitSourceLocExpr(SourceLocExpr *E) {
+ HasImmediateCalls = true;
+ return RecursiveASTVisitor<ImmediateCallVisitor>::VisitStmt(E);
----------------
It may be worth a comment that this relies on the behavior of `VisitLambdaExpr`
not changing to visit the lambda body. Otherwise, nested invocations would be
an issue in something like `void func(int a = []{ return
std::source_location::current(); }());` because we'd claim there is an
immediate call for that default arg expression when there isn't one.
As it stands, I wonder what the behavior of this code should be:
```
void func(int a =
({
int val = std::souce_location::current();
val;
})
);
```
(Oh, wait, I know, I think we made that code invalid because GNU statement
expressions in a default argument are horrifying...)
Also, it looks like this code ignores blocks while handling lambdas; shouldn't
blocks be handled the same way as lambdas?
Can you also add a test for this example:
```
void func(int a =
(int){ std::source_location::current() }
);
```
and I suppose we also should have a test for:
```
consteval int terrible_idea_dont_write_this_code_please(
int &out,
const int (&a)[std::source_location::current()] = {
std::source_location::current() }
) { out = a[0]; return sizeof(a); }
consteval void func() {
int a;
static_assert(terrible_idea_dont_write_this_code_please(a) == a, "hahaha, not
a chance!");
}
```
which demonstrates just how deeply bizarre WG21's decision is here -- the
returned value != `out`.
And similar tests for default inits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136554/new/
https://reviews.llvm.org/D136554
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
