[PATCH] D149276: [Clang] Fix parsing of `(auto(x))`.

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 27 05:42:27 PDT 2023


aaron.ballman added inline comments.


================
Comment at: clang/lib/Parse/ParseTentative.cpp:1067
+                                            bool mayHaveDirectInit,
+                                            bool mayHaveTrailingReturnType) {
   // declarator:
----------------
tbaeder wrote:
> Not part of this commit I guess, but //four// bool parameters is truly horrific :/
Yeah, a refactoring here would not be amiss (but outside of this patch, I think).


================
Comment at: clang/lib/Parse/ParseTentative.cpp:1416-1417
+  case tok::kw_auto: {
+    if (NextToken().is(tok::l_brace))
+      return TPResult::False;
+    if (NextToken().is(tok::l_paren)) {
----------------
aaron.ballman wrote:
> This seems incorrect to me, consider:
> ```
> auto [[clang::annotate_type("test")]] i = 12;
> ```
> https://godbolt.org/z/7Gx3Gb18h
Ignore this -- `l_brace` is not `l_square`. :-D


================
Comment at: clang/test/Parser/cxx1z-decomposition.cpp:97
     decltype(auto) [b] = s; // expected-error {{cannot be declared with type 'decltype(auto)'}}
-    auto ([c]) = s; // expected-error {{cannot be declared with parentheses}}
+    auto ([c]) = s; // expected-error {{'auto' not allowed here}} \
+                    // expected-error {{use of undeclared identifier 'c'}} \
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > This first diagnostic is incorrect -- `auto` is definitely allowed there, just that the rest of the declaration is nonsense. I think the "use of undeclared identifier" diagnostic is reasonable. The lambda diagnostic is.... interesting.
> > 
> > Any way you can improve this, or is that a slog?
> There were multiple issues there.
> 
> The "decomposition declaration cannot be declared with parenthese" error which i had remove can still occur at global scope where a declaration is expected so i put that back and added a test.
> Then, I'm running all the tests in c++17 and 23 modes as the behavior is different.
> 
> I did restore the current behavior in C++20 and earlier modes, such that we will always consider `auto` starts a declaration. That way we can have that structured binding in parentheses diag.
> 
> But in c++23 mode,  We establish it's not a valid declarator. so we parse it as an expression. `[` is always considered as a lambda introducer, which it could be, ie `auto([c] {});` is a valid expression assuming that C exists.
> At that point trying to figure out that `]` is not followed by `{` and is therefore not a lambda seems extremely tricky as you can have arbitrary long expressions between the square brackets.
> 
Thanks for the explanation -- that seems defensible to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149276



More information about the cfe-commits mailing list