[PATCH] D111175: [Clang] Extend init-statement to allow alias-declaration

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: erichkeane, rsmith.
aaron.ballman added a comment.

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?



================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:553
+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<
----------------
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.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:555
+def ext_alias_in_init_statement : ExtWarn<
+  "alias declaration in init statements is a C++2b extension">,
+  InGroup<CXX2b>;
----------------
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.


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1924-1928
+  const Decl *InitDecl = DG.get().getSingleDecl();
+  if (!isa<TypeAliasDecl>(InitDecl)) {
+    Diag(DeclStart, diag::err_expected_alias_after_using_in_init_statement)
+        << SourceRange(DeclStart, DeclEnd);
+  }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111175