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