[Clang] Extend init-statement to allow alias-declaration

[cor3ntin]

Summary

Implement P2360R0 in C++23 mode and as an extension in older
languages mode.

[aaron.ballman]

Thank you for this new feature work! Adding a few more reviewers to the mix.

Can you also add a release note for the new functionality?

clang/include/clang/Basic/DiagnosticParseKinds.td

553

This diagnostic confuses me. An init-statement can be an expression-statement, simple-declaration, or alias-declaration. So if we're in an init-statement and see a using keyword, the only think we can be parsing is an alias declaration, right?

I guess I would have expected that we'd eventually wind up calling Parser::ParseAliasDeclarationAfterDeclarator() via ParseUsingDeclaration() and that would handle any parsing-related concerns with the construct.

Beyond that -- users don't really know what an init statement is, I think we'd usually call this a "condition" instead of an init statement in diagnostics.

555

Same concern here about "init statements", we probably should list the constructs specifically.

Also, I think we're missing the "is incompatible with standards before" variant of this extension diagnostic.

clang/lib/Parse/ParseExprCXX.cpp

1924–1928

Okay, to verify my understanding of the logic here: you call ParseUsingDeclaration(), but that may parse a using declaration or an alias declaration, so you need to handle the case where it parsed something that's not allowed.

Assuming that's correct, I think a better approach is for ParseUsingDeclaration() to pay attention to the DeclaratorContext it is given, and when it's an init statement, refuse to parse anything other than an alias declaration. Then you can get rid of err_expected_alias_after_using_in_init_statement entirely, as the user will get the typical parsing errors. If those diagnostics are low quality and we need a custom diagnostic instead, that logic can live in ParseUsingDeclaration() instead.

That just about obviates the need for ParseAliasDeclarationInInitStatement() entirely, but this function still adds value because of the extension/precompat warnings so I think it's worth keeping around.

[rsmith]

clang/include/clang/Basic/DiagnosticParseKinds.td

553

I think there's a more fundamental problem with this diagnostic: it's a compiler-oriented explanation of what went wrong *for us*, not a user-oriented explanation of what's wrong *with their code*. If the user wrote if (using X::A; A a = b) or if (using namespace N; ...), telling them that they could have written an alias declaration instead is not helpful -- that has no relevance to the problem they're trying to solve. It'd be better to say "using [namespace] declaration cannot appear here" and not mention alias declarations at all.

555

We could use a %select to specify the context instead of saying "init statement", but actually describing the contexts seems hard to do succinctly and clearly. In this case I think it'd be OK to just say "in this context" or similar if we don't want to mention init-statements. (As far as I can tell, literally no-one outside a C++ parser's test suite has ever put a typedef in an init-statement, so we should similarly assume this feature will never be used outside our own tests. The %select is probably not worth the effort.)

[erichkeane]

I agree with aaron, the ParseUsingDeclaration bit should be aware of the DeclaratorContext and emit the error there, rather than try to parse it as whatever and THEN error.

The problem is the user-interface at that point, which is to spend a few cycles correcting one of the other forms, just to then tell them it isn't permitted. This is a case where erroring 'early' in this case would be a much better user experience.

clang/include/clang/Parse/Parser.h

2401

Slight preference for reversing these conditions

clang/lib/Parse/ParseExprCXX.cpp

2047

single line if/else don't get '{' or '}'.

[cor3ntin]

clang/include/clang/Basic/DiagnosticParseKinds.td

553

We do but ParseUsingDeclaration can parse other things, which are not allowed there.
I will try to modify ParseUsingDeclaration, which might not lead to better diagnostics, but at least won't be compiler-oriented.

555

This was my justification for not doing more work. I agree that "in this context" is better

clang/lib/Parse/ParseExprCXX.cpp

1924–1928

Yes, I considered modifying ParseUsingDeclaration but wanted to keep the change somewhat localized, and keep ParseUsingDeclaration as is on the basis that no other place in the standard only accept alias-declaration. But the solution you suggest is certainly possible.
I don't know how much better the diagnostic would be.

[cor3ntin]

• Modify ParseUsingDeclaration to only parse an alias in init statememts
• Make the c++23 extebsion warning clearer
• Nitpicks.

The error is now 'expected '=', which seems good enough in that scenario.

[cor3ntin]

Update release notes

[aaron.ballman]

clang/include/clang/Basic/DiagnosticParseKinds.td

552–553

Can you also add the "incompatible with standards before C++2b" variant of this diagnostic?

clang/lib/Parse/ParseDeclCXX.cpp

720–722 ↗

(On Diff #377756)

clang/lib/Parse/ParseExprCXX.cpp

1927–1930

Though I suspect this will change somewhat when you add the precompat warning.

2050–2053

clang/lib/Parse/ParseTentative.cpp

486–489

No else after a return, and can drop the curly braces, per coding standard.

[cor3ntin]

Add C++20 compat warning, fix style.

[erichkeane]

clang/test/Parser/cxx2b-init-statement.cpp

20

Is there any way to have this elaborate more? The 'expected 'TOKEN'' is my least favorite type of error. Could we change this to be something like, 'expected '=' in type alias' or something?

[aaron.ballman]

Just nits from me, but otherwise LGTM!

clang/include/clang/Basic/DiagnosticParseKinds.td

556

clang/include/clang/Parse/Parser.h

1982

clang/lib/Parse/ParseExprCXX.cpp

1915

clang/test/Parser/cxx2b-init-statement.cpp

20

FWIW, I'm fine with the diagnostic as-is because I don't expect anyone to actually *use* this functionality, so I don't expect a ton of users to hit this. I'd say it's fine to address when a user files a bug showing some code where the diagnostic is unclear.

[cor3ntin]

fix diagnostic, nits

[cor3ntin]

Fix version in diagnostics in the right direction (23 => 2b)

[erichkeane]

clang/test/Parser/cxx2b-init-statement.cpp

20

I'm sad by this, and Aaron points out it is a bit of a hassle, so I can disagree & commit here.

[cor3ntin]

clang/test/Parser/cxx2b-init-statement.cpp

20

Yeah, i guess we could but i really do not expect that feature to have a lot of uses. So i tried to keep the patch minimal.

[erichkeane]

clang/test/Parser/cxx2b-init-statement.cpp

20

Aaron and I discussed it, and I'm willing to relent, so feel free to land. In Aaron's opinion right now:

Effort to improve diagnostic >(greatly) Chance of someone running into it

I'm willing to wait until there is proof of the RHS of that equation being of greater magnitude before being willing to ask for the effort on the LHS.

[aaron.ballman]

I've commit on your behalf in ff013b61004b01043bdbebb6416c30ecb3d3d48c, thank you for the new functionality!

clang/include/clang/Basic/DiagnosticParseKinds.td

 def err_expected_lparen_after_type : Error<
   "expected '(' for function-style cast or type construction">;
 def err_expected_init_in_condition : Error<
   "variable declaration in condition must have an initializer">;
 def err_expected_init_in_condition_lparen : Error<
   "variable declaration in condition cannot have a parenthesized initializer">;
 def err_extraneous_rparen_in_condition : Error<
   "extraneous ')' after condition, expected a statement">;
+def err_expected_alias_after_using_in_init_statement : Error<
+  "expected alias declaration after using in init statement">;
+def ext_alias_in_init_statement : ExtWarn<
+  "alias declaration in init statements is a C++2b extension">,
+  InGroup<CXX2b>;
 def warn_dangling_else : Warning<
   "add explicit braces to avoid dangling else">,
   InGroup<DanglingElse>;
 def err_expected_member_or_base_name : Error<
   "expected class member or base class name">;
 def err_expected_lbrace_after_base_specifiers : Error<
   "expected '{' after base class list">;
 def err_missing_end_of_definition : Error<

clang/include/clang/Parse/Parser.h

private:
   //===--------------------------------------------------------------------===//
   // C++ if/switch/while/for condition expression.
   struct ForRangeInfo;
   Sema::ConditionResult ParseCXXCondition(StmtResult *InitStmt,
                                           SourceLocation Loc,
                                           Sema::ConditionKind CK,
                                           ForRangeInfo *FRI = nullptr,
                                           bool EnterForConditionScope = false);
+  DeclGroupPtrTy
+  ParseAliasDeclarationInInitStatement(ParsedAttributesWithRange &attrs);
 
   //===--------------------------------------------------------------------===//
   // C++ Coroutines
 
   ExprResult ParseCoyieldExpression();
 
   //===--------------------------------------------------------------------===//
   // C++ Concepts
 
private:
   /// isForInitDeclaration - Disambiguates between a declaration or an
   /// expression in the context of the C 'clause-1' or the C++
   // 'for-init-statement' part of a 'for' statement.
   /// Returns true for declaration, false for expression.
   bool isForInitDeclaration() {
     if (getLangOpts().OpenMP)
       Actions.startOpenMPLoop();
     if (getLangOpts().CPlusPlus)
-      return isCXXSimpleDeclaration(/*AllowForRangeDecl=*/true);
+      return isCXXSimpleDeclaration(/*AllowForRangeDecl=*/true) ||
+             Tok.is(tok::kw_using);
     return isDeclarationSpecifier(true);
   }
 
   /// Determine whether this is a C++1z for-range-identifier.
   bool isForRangeIdentifier();
 
   /// Determine whether we are currently at the start of an Objective-C
   /// class message that appears to be missing the open bracket '['.

clang/lib/Parse/ParseExprCXX.cpp

if (Tok.is(tok::l_brace)) {
     assert((Exprs.size() == 0 || Exprs.size()-1 == CommaLocs.size())&&
            "Unexpected number of commas!");
     return Actions.ActOnCXXTypeConstructExpr(TypeRep, T.getOpenLocation(),
                                              Exprs, T.getCloseLocation(),
                                              /*ListInitialization=*/false);
   }
 }
 
+Parser::DeclGroupPtrTy
+Parser::ParseAliasDeclarationInInitStatement(ParsedAttributesWithRange &attrs) {
+  assert(Tok.is(tok::kw_using) && "Expected using");
+  DeclGroupPtrTy DG;
+  SourceLocation DeclStart = ConsumeToken(), DeclEnd;
+
+  DG = ParseUsingDeclaration(DeclaratorContext::SelectionInit, {}, DeclStart,
+                             DeclEnd, attrs, AS_none);
+  if (!DG)
+    return DG;
+
+  const Decl *InitDecl = DG.get().getSingleDecl();
+  if (!isa<TypeAliasDecl>(InitDecl)) {
+    Diag(DeclStart, diag::err_expected_alias_after_using_in_init_statement)
+        << SourceRange(DeclStart, DeclEnd);
+  }
+
+  if (!getLangOpts().CPlusPlus2b) {
+    Diag(DeclStart, diag::ext_alias_in_init_statement)
+        << SourceRange(DeclStart, DeclEnd);
+  }
+
+  return DG;
+}
+
 /// ParseCXXCondition - if/switch/while condition expression.
 ///
 ///       condition:
 ///         expression
 ///         type-specifier-seq declarator '=' assignment-expression
 /// [C++11] type-specifier-seq declarator '=' initializer-clause
 /// [C++11] type-specifier-seq declarator braced-init-list
 /// [Clang] type-specifier-seq ref-qualifier[opt] '[' identifier-list ']'
if (InitStmt && Tok.is(tok::semi)) {
       return ParseCXXCondition(nullptr, Loc, CK);
     }
 
     return Actions.ActOnCondition(getCurScope(), Loc, Expr.get(), CK);
   }
 
   case ConditionOrInitStatement::InitStmtDecl: {
     WarnOnInit();
+    DeclGroupPtrTy DG;
     SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
-    DeclGroupPtrTy DG = ParseSimpleDeclaration(
-        DeclaratorContext::SelectionInit, DeclEnd, attrs, /*RequireSemi=*/true);
+    if (Tok.is(tok::kw_using)) {
+      DG = ParseAliasDeclarationInInitStatement(attrs);
+    } else {
+      DG = ParseSimpleDeclaration(DeclaratorContext::SelectionInit, DeclEnd,
+                                  attrs, /*RequireSemi=*/true);
+    }
     *InitStmt = Actions.ActOnDeclStmt(DG, DeclStart, DeclEnd);
     return ParseCXXCondition(nullptr, Loc, CK);
   }
 
   case ConditionOrInitStatement::ForRangeDecl: {
     // This is 'for (init-stmt; for-range-decl : range-expr)'.
     // We're not actually in a for loop yet, so 'break' and 'continue' aren't
     // permitted here.
     assert(FRI && "should not parse a for range declaration here");

clang/lib/Parse/ParseStmt.cpp

 ///             '(' for-range-declaration ':' for-range-initializer ')'
 ///             statement
 /// [OBJC2] 'for' '(' declaration 'in' expr ')' statement
 /// [OBJC2] 'for' '(' expr 'in' expr ')' statement
 ///
 /// [C++] for-init-statement:
 /// [C++]   expression-statement
 /// [C++]   simple-declaration
+/// [C++2b] alias-declaration
 ///
 /// [C++0x] for-range-declaration:
 /// [C++0x]   attribute-specifier-seq[opt] type-specifier-seq declarator
 /// [C++0x] for-range-initializer:
 /// [C++0x]   expression
 /// [C++0x]   braced-init-list            [TODO]
 StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
   assert(Tok.is(tok::kw_for) && "Not a for stmt!");
StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
   } else if (isForInitDeclaration()) {  // for (int X = 4;
     ParenBraceBracketBalancer BalancerRAIIObj(*this);
 
     // Parse declaration, which eats the ';'.
     if (!C99orCXXorObjC) {   // Use of C99-style for loops in C90 mode?
       Diag(Tok, diag::ext_c99_variable_decl_in_for_loop);
       Diag(Tok, diag::warn_gcc_variable_decl_in_for_loop);
     }
-
+    DeclGroupPtrTy DG;
+    if (Tok.is(tok::kw_using)) {
+      DG = ParseAliasDeclarationInInitStatement(attrs);
+    } else {
       // In C++0x, "for (T NS:a" might not be a typo for ::
       bool MightBeForRangeStmt = getLangOpts().CPlusPlus;
       ColonProtectionRAIIObject ColonProtection(*this, MightBeForRangeStmt);
 
       SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
-    DeclGroupPtrTy DG = ParseSimpleDeclaration(
+      DG = ParseSimpleDeclaration(
           DeclaratorContext::ForInit, DeclEnd, attrs, false,
           MightBeForRangeStmt ? &ForRangeInfo : nullptr);
       FirstPart = Actions.ActOnDeclStmt(DG, DeclStart, Tok.getLocation());
       if (ForRangeInfo.ParsedForRangeDecl()) {
-      Diag(ForRangeInfo.ColonLoc, getLangOpts().CPlusPlus11 ?
-           diag::warn_cxx98_compat_for_range : diag::ext_for_range);
+        Diag(ForRangeInfo.ColonLoc, getLangOpts().CPlusPlus11
+                                        ? diag::warn_cxx98_compat_for_range
+                                        : diag::ext_for_range);
         ForRangeInfo.LoopVar = FirstPart;
         FirstPart = StmtResult();
-    } else if (Tok.is(tok::semi)) {  // for (int x = 4;
+      } else if (Tok.is(tok::semi)) { // for (int x = 4;
         ConsumeToken();
       } else if ((ForEach = isTokIdentifier_in())) {
         Actions.ActOnForEachDeclStmt(DG);
         // ObjC: for (id x in expr)
         ConsumeToken(); // consume 'in'
 
         if (Tok.is(tok::code_completion)) {
           cutOffParsing();
           Actions.CodeCompleteObjCForCollection(getCurScope(), DG);
           return StmtError();
         }
         Collection = ParseExpression();
       } else {
         Diag(Tok, diag::err_expected_semi_for);
       }
+    }
   } else {
     ProhibitAttributes(attrs);
     Value = Actions.CorrectDelayedTyposInExpr(ParseExpression());
 
     ForEach = isTokIdentifier_in();
 
     // Turn the expression into a stmt.
     if (!Value.isInvalid()) {

clang/lib/Parse/ParseTentative.cpp

 /// to the ';' to disambiguate cases like 'int(x))' (an expression) from
 /// 'int(x);' (a simple-declaration in an init-statement).
 Parser::ConditionOrInitStatement
 Parser::isCXXConditionDeclarationOrInitStatement(bool CanBeInitStatement,
                                                  bool CanBeForRangeDecl) {
   ConditionDeclarationOrInitStatementState State(*this, CanBeInitStatement,
                                                  CanBeForRangeDecl);
 
-  if (State.update(isCXXDeclarationSpecifier()))
+  if (CanBeInitStatement && Tok.is(tok::kw_using)) {
+    return ConditionOrInitStatement::InitStmtDecl;
+  } else if (State.update(isCXXDeclarationSpecifier()))
     return State.result();
 
   // It might be a declaration; we need tentative parsing.
   RevertingTentativeParsingAction PA(*this);
 
   // FIXME: A tag definition unambiguously tells us this is an init-statement.
   if (State.update(TryConsumeDeclarationSpecifier()))
     return State.result();
   assert(Tok.is(tok::l_paren) && "Expected '('");

clang/test/Parser/cxx2b-init-statement.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify=expected -std=c++2b -Wall %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,expected-cxx20 -std=c++20 -Wall %s
+
+namespace ns {
+    int i;
+    enum class e {};
+}
+void f() {
+
+    for (using foo = int;true;); //expected-cxx20-warning {{alias declaration in init statements is a C++2b extension}}
+
+    switch(using foo = int; 0) { //expected-cxx20-warning {{alias declaration in init statements is a C++2b extension}}
+        case 0: break;
+    }
+
+    if(using foo = int; false) {} //expected-cxx20-warning {{alias declaration in init statements is a C++2b extension}}
+
+
+    if (using enum ns::e; false){}  // expected-error {{expected alias declaration after using in init statement}} \
+                                    // expected-cxx20-warning {{alias declaration in init statements is a C++2b extension}}
+
+
+    for (using ns::i; true;);  // expected-error {{expected alias declaration after using in init statement}} \
+                               // expected-cxx20-warning {{alias declaration in init statements is a C++2b extension}}
+
+    if (using ns::i; false){}  // expected-error {{expected alias declaration after using in init statement}} \
+                               // expected-cxx20-warning {{alias declaration in init statements is a C++2b extension}}
+
+    switch(using ns::i; 0) {   // expected-error {{expected alias declaration after using in init statement}} \
+                               // expected-cxx20-warning {{alias declaration in init statements is a C++2b extension}}
+        case 0: break;
+    }
+
+}

clang/test/SemaCXX/cxx2b-init-statement.cpp

+// RUN: %clang_cc1 -verify -std=c++2b -Wall -Wshadow %s
+
+void f() {
+
+    for (using foo = int;true;) {} //expected-warning {{unused type alias 'foo'}}
+
+    switch(using foo = int; 0) { //expected-warning {{unused type alias 'foo'}}
+        case 0: break;
+    }
+
+    if(using foo = int; false) {} // expected-warning {{unused type alias 'foo'}}
+
+    int x; // expected-warning {{unused variable 'x'}}
+    if(using x = int; true) {}  // expected-warning {{unused type alias 'x'}}
+
+    using y = int; // expected-warning {{unused type alias 'y'}} \
+                   // expected-note 2{{previous declaration is here}}
+
+    if(using y = double; true) {}  // expected-warning {{unused type alias 'y'}} \
+                                   // expected-warning {{declaration shadows a type alias in function 'f'}}
+
+    for(using y = double; true;) { // expected-warning {{declaration shadows a type alias in function 'f'}}
+        y foo = 0;
+        (void)foo;
+        constexpr y var = 0;
+        static_assert(var == 0);
+    }
+}

clang/www/cxx_status.html

</tr>
     <tr>
       <td>Add support for preprocessing directives elifdef and elifndef</td>
       <td><a href="https://wg21.link/P2334R1">P2334R1</a></td>
       <td class="full" align="center">Clang 13</td>
     </tr>
     <tr>
       <td>Extend init-statement to allow alias-declaration</td>
       <td><a href="https://wg21.link/P2360R0">P2360R0</a></td>
-      <td class="none" align="center">No</td>
+      <td class="unreleased" align="center">Clang 14</td>
     </tr>
 
 </table>
 </details>
 
 <h2 id="dr">Defect reports</h2>
 
 <p>Clang generally aims to implement resolutions to Defect Reports (bug fixes