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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
