aaron.ballman added a comment.
In D119609#3426279 <https://reviews.llvm.org/D119609#3426279>, @erichkeane
wrote:
> I think LGTM on technical perspective, Please don't commit until
> @aaron.ballman confirms he is OK with the direction, or would like to wait
> longer for GCC.
I spotted some technical issues with the diagnostic wording. As for waiting for
GCC, my concern there is largely with the fact that people use statement
expressions in macros so that they can define local variables that don't impact
the outer scope where the macro is expanded. I could see such a macro being
used as a default argument, so I worry a bit about breaking code in that case.
However, C doesn't have default arguments, so this isn't an issue there. And
C++ has lambdas which can be called in a default argument, so users have some
ability to fix their code in the event of an error so long as they're in C++11
or later. Because of that, I would be okay moving forward with this, but
cautiously in case someone reports code that breaks and can't easily be fixed
(or significant breakage in an important 3rd party header).
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4383-4384
"%select{default|copy|move}0 constructor">;
+def err_expr_statement_in_default_arg : Error<
+ "expression statement not permitted in default argument">;
----------------
I get mixed up every time, but I double-checked -- these are statement
expressions, not expression statements. I also think the diagnostic should be
more clear about default argument vs default non-type template argument.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:7073
+ Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+ // Skip the expression statement and continue parsing
+ SkipUntil(tok::comma, StopBeforeMatch);
----------------
================
Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:1
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++20
+
----------------
You should rename this test to `stmt-expr-in-default-arg.cpp`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119609/new/
https://reviews.llvm.org/D119609
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
