[PATCH] D39284: Allow conditions to be decomposed with structured bindings
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 30 17:27:40 PST 2017
rsmith added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:414
+def ext_decomp_decl_cond : ExtWarn<
+ "decomposed condition is a Clang extension">;
def err_decomp_decl_spec : Error<
----------------
Phrase this as "ISO C++17 does not permit structured binding declaration in a condition"
================
Comment at: lib/Sema/SemaDeclCXX.cpp:712-720
+ Diag(Decomp.getLSquareLoc(), [&] {
+ if (getLangOpts().CPlusPlus1z) {
+ if (D.getContext() == Declarator::ConditionContext)
+ return diag::ext_decomp_decl_cond;
+ else
+ return diag::warn_cxx14_compat_decomp_decl;
+ } else
----------------
Using a lambda here seems like unwarranted complexity. A three-way ternary of the form
```
!getLangOpts().CPlusPlus1z ? diag::ext_decomp_decl :
D.getContext() == Declarator::ConditionContext ? diag::ext_decomp_decl_cond :
diag::warn_cxx14_compat_decomp_decl
```
would be fine. Feel free to ignore clang-format if it wants to format it stupidly.
================
Comment at: test/Misc/warning-flags.c:19
The list of warnings below should NEVER grow. It should gradually shrink to 0.
----------------
Please read and respect this rule :)
================
Comment at: test/Parser/cxx1z-decomposition.cpp:35-36
void g() {
- // A condition is not a simple-declaration.
- for (; auto [a, b, c] = S(); ) {} // expected-error {{not permitted in this context}}
- if (auto [a, b, c] = S()) {} // expected-error {{not permitted in this context}}
- if (int n; auto [a, b, c] = S()) {} // expected-error {{not permitted in this context}}
- switch (auto [a, b, c] = S()) {} // expected-error {{not permitted in this context}}
- switch (int n; auto [a, b, c] = S()) {} // expected-error {{not permitted in this context}}
- while (auto [a, b, c] = S()) {} // expected-error {{not permitted in this context}}
+ // A condition is allowed as a Clang extension.
+ // See commentary in test/Parser/decomposed-condition.cpp
----------------
You have removed test coverage here, at least for the `if` //init-statement// case. Please just update the comments on these tests rather than removing them.
https://reviews.llvm.org/D39284
More information about the cfe-commits
mailing list