CWG2635: Disallow constrained structured bindings.

[erichkeane]

Summary

CWG2635 prohibits adding a constraint to a structured as a defect
report. This patch implements that restriction.

[erichkeane]

I would normally not really need a review here, but I did some stuff with the fixit that I'm not sure about/not sure how to test, so I was hoping someone could take a second look

[aaron.ballman]

The changes look correct to me. The way to test the fix-it behavior is to model after one of the tests in https://github.com/llvm/llvm-project/tree/main/clang/test/FixIt (there is -fdiagnostics-parseable-fixits and friends for this purpose).

clang/include/clang/Basic/DiagnosticSemaKinds.td

495–496

I think clang-format butchered your formatting, you should use the more typical style.

clang/lib/Sema/SemaDeclCXX.cpp

765–768

[aaron.ballman]

LGTM, but please add a release note when landing.

clang/docs/ReleaseNotes.rst

- Implemented `P2113R0: Proposed resolution for 2019 comment CA 112 <https://wg21.link/P2113R0>`_
   ([temp.func.order]p6.2.1 is not implemented, matching GCC).
 - Implemented `P0857R0 <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0857r0.html>`_,
   which specifies constrained lambdas and constrained template *template-parameter*\s.
 - Required parameter pack to be provided at the end of the concept parameter list. This
   fixes `Issue 48182 <https://github.com/llvm/llvm-project/issues/48182>`_.
 
 - Do not hide templated base members introduced via using-decl in derived class
   (useful specially for constrained members). Fixes `GH50886 <https://github.com/llvm/llvm-project/issues/50886>`_.
+- Implemented CWG2635 as a Defect Report, which prohibits structured bindings from being constrained.
 
 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Support label at end of compound statement (`P2324 <https://wg21.link/p2324r2>`_).
 - Implemented `P1169R4: static operator() <https://wg21.link/P1169R4>`_.
 - Implemented "char8_t Compatibility and Portability Fix" (`P2513R3 <https://wg21.link/P2513R3>`_).
   This change was applied to C++20 as a Defect Report.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def warn_cxx17_compat_decomp_decl_spec : Warning<
   "decomposition declaration declared "
   "%plural{1:'%1'|:with '%1' specifiers}0 "
   "is incompatible with C++ standards before C++20">,
   InGroup<CXXPre20Compat>, DefaultIgnore;
 def err_decomp_decl_type : Error<
   "decomposition declaration cannot be declared with type %0; "
   "declared type must be 'auto' or reference to 'auto'">;
+def err_decomp_decl_constraint : Error<
+  "decomposition declaration cannot be declared with constrained 'auto'">;
 def err_decomp_decl_parens : Error<
   "decomposition declaration cannot be declared with parentheses">;
 def err_decomp_decl_template : Error<
   "decomposition declaration template not supported">;
 def err_decomp_decl_not_alone : Error<
   "decomposition declaration must be the only declaration in its group">;
 def err_decomp_decl_requires_init : Error<
   "decomposition declaration %0 requires an initializer">;

clang/lib/Sema/SemaDeclCXX.cpp

Diag(Decomp.getLSquareLoc(),
       << Decomp.getSourceRange();
 
   // The semantic context is always just the current context.
   DeclContext *const DC = CurContext;
 
   // C++17 [dcl.dcl]/8:
   //   The decl-specifier-seq shall contain only the type-specifier auto
   //   and cv-qualifiers.
-  // C++2a [dcl.dcl]/8:
+  // C++20 [dcl.dcl]/8:
   //   If decl-specifier-seq contains any decl-specifier other than static,
   //   thread_local, auto, or cv-qualifiers, the program is ill-formed.
+  // C++2b [dcl.pre]/6:
+  //   Each decl-specifier in the decl-specifier-seq shall be static,
+  //   thread_local, auto (9.2.9.6 [dcl.spec.auto]), or a cv-qualifier.
   auto &DS = D.getDeclSpec();
   {
+    // Note: While constrained-auto needs to be checked, we do so separately so
+    // we can emit a better diagnostic.
     SmallVector<StringRef, 8> BadSpecifiers;
     SmallVector<SourceLocation, 8> BadSpecifierLocs;
     SmallVector<StringRef, 8> CPlusPlus20Specifiers;
     SmallVector<SourceLocation, 8> CPlusPlus20SpecifierLocs;
     if (auto SCS = DS.getStorageClassSpec()) {
       if (SCS == DeclSpec::SCS_static) {
         CPlusPlus20Specifiers.push_back(DeclSpec::getSpecifierName(SCS));
         CPlusPlus20SpecifierLocs.push_back(DS.getStorageClassSpecLoc());
if (DS.hasConstexprSpecifier()) {
       BadSpecifiers.push_back(
           DeclSpec::getSpecifierName(DS.getConstexprSpecifier()));
       BadSpecifierLocs.push_back(DS.getConstexprSpecLoc());
     }
     if (DS.isInlineSpecified()) {
       BadSpecifiers.push_back("inline");
       BadSpecifierLocs.push_back(DS.getInlineSpecLoc());
     }
+
     if (!BadSpecifiers.empty()) {
       auto &&Err = Diag(BadSpecifierLocs.front(), diag::err_decomp_decl_spec);
       Err << (int)BadSpecifiers.size()
           << llvm::join(BadSpecifiers.begin(), BadSpecifiers.end(), " ");
       // Don't add FixItHints to remove the specifiers; we do still respect
       // them when building the underlying variable.
       for (auto Loc : BadSpecifierLocs)
         Err << SourceRange(Loc, Loc);
if (DS.getTypeSpecType() != DeclSpec::TST_auto ||
 
     // In most cases, there's no actual problem with an explicitly-specified
     // type, but a function type won't work here, and ActOnVariableDeclarator
     // shouldn't be called for such a type.
     if (R->isFunctionType())
       D.setInvalidType();
   }
 
+  // Constrained auto is prohibited by [decl.pre]p6, so check that here.
+  if (DS.isConstrainedAuto()) {
+    TemplateIdAnnotation *TemplRep = DS.getRepAsTemplateId();
+    assert(TemplRep->Kind == TNK_Concept_template &&
+           "No other template kind should be possible for a constrained auto");
+
+    SourceRange TemplRange{TemplRep->TemplateNameLoc,
+                           TemplRep->RAngleLoc.isValid()
+                               ? TemplRep->RAngleLoc
+                               : TemplRep->TemplateNameLoc};
+    Diag(TemplRep->TemplateNameLoc, diag::err_decomp_decl_constraint)
+        << TemplRange << FixItHint::CreateRemoval(TemplRange);
+  }
+
   // Build the BindingDecls.
   SmallVector<BindingDecl*, 8> Bindings;
 
   // Build the BindingDecls.
   for (auto &B : D.getDecompositionDeclarator().bindings()) {
     // Check for name conflicts.
     DeclarationNameInfo NameInfo(B.Name, B.NameLoc);
     LookupResult Previous(*this, NameInfo, LookupOrdinaryName,

clang/test/CXX/drs/dr26xx.cpp

 
 void f() {
   foo fooable; // expected-error {{call to deleted}}
   // expected-note@#DR2628_CTOR {{marked deleted here}}
 }
 
 }
 
+namespace dr2635 { // dr2635: yes
+template<typename T>
+concept UnaryC = true;
+template<typename T, typename U>
+concept BinaryC = true;
+
+struct S{ int i, j; };
+S get_S();
+
+template<typename T>
+T get_T();
+
+void use() {
+  // expected-error@+1{{decomposition declaration cannot be declared with constrained 'auto'}}
+  UnaryC auto [a, b] = get_S();
+  // expected-error@+1{{decomposition declaration cannot be declared with constrained 'auto'}}
+  BinaryC<int> auto [c, d] = get_S();
+}
+
+template<typename T>
+void TemplUse() {
+  // expected-error@+1{{decomposition declaration cannot be declared with constrained 'auto'}}
+  UnaryC auto [a, b] = get_T<T>();
+  // expected-error@+1{{decomposition declaration cannot be declared with constrained 'auto'}}
+  BinaryC<T> auto [c, d] = get_T<T>();
+}
+}
+
   // dr2636: na
 
   // dr2642: na
 
 namespace dr2644 { // dr2644: yes
 
 auto z = [a = 42](int a) { // expected-error {{a lambda parameter cannot shadow an explicitly captured entity}} \
                            // expected-note {{variable 'a' is explicitly captured here}}

clang/test/FixIt/fixit-constrained-structured-binding.cpp

+// RUN: not %clang_cc1 -std=c++20 -fdiagnostics-parseable-fixits -x c++ %s 2> %t
+// RUN: FileCheck %s < %t
+
+template<typename T>
+concept UnaryC = true;
+template<typename T, typename U>
+concept BinaryC = true;
+
+struct S{ int i, j; };
+S get_S();
+
+template<typename T>
+T get_T();
+
+void use() {
+  UnaryC auto [a, b] = get_S();
+  // CHECK: error: decomposition declaration cannot be declared with constrained 'auto'
+  // CHECK: fix-it:{{.*}}:{16:3-16:10}:""
+  BinaryC<int> auto [c, d] = get_S();
+  // CHECK: error: decomposition declaration cannot be declared with constrained 'auto'
+  // CHECK: fix-it:{{.*}}:{19:3-19:16}:""
+}
+
+template<typename T>
+void TemplUse() {
+  UnaryC auto [a, b] = get_T<T>();
+  // CHECK: error: decomposition declaration cannot be declared with constrained 'auto'
+  // XCHECK: fix-it:{{.*}}:{26:3-26:10}:""
+  BinaryC<T> auto [c, d] = get_T<T>();
+  // CHECK: error: decomposition declaration cannot be declared with constrained 'auto'
+  // XCHECK: fix-it:{{.*}}:{29:3-29:14}:""
+}
+

clang/www/cxx_dr_status.html

<tr class="open" id="2634">
     <td>open</td>
     <td>Avoid circularity in specification of scope for friend class declarations</td>
     <td align="center">Not resolved</td>
   </tr>
   <tr id="2635">
     <td><a href="https://wg21.link/cwg2635">2635</a></td>
     <td>DR</td>
     <td>Constrained structured bindings</td>
-    <td class="none" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 16</td>
   </tr>
   <tr id="2636">
     <td><a href="https://wg21.link/cwg2636">2636</a></td>
     <td>DR</td>
     <td>Update Annex E based on Unicode 15.0 UAX #31</td>
     <td class="na" align="center">N/A</td>
   </tr>
   <tr class="open" id="2637">