[PATCH] D136554: Implement CWG2631

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:5927
+  bool VisitSourceLocExpr(SourceLocExpr *E) {
+    HasImmediateCalls = true;
+    return RecursiveASTVisitor<ImmediateCallVisitor>::VisitStmt(E);
----------------
aaron.ballman wrote:
> 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.
Another test case I devised shows we've got a lambda bug in clang, but would be worth reasoning about:
```
void func(int a = [b = std::source_location::current()] { return b; }());
```
(this should be valid, but I have no idea what value `a` should have if used as a default argument.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136554/new/

https://reviews.llvm.org/D136554