[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