rsmith added inline comments.

================
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<
----------------
aaron.ballman wrote:
> 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.
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.


================
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>;
----------------
aaron.ballman wrote:
> 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.
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.)


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

Reply via email to