Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang] Implement C++26 Attributes for Structured Bindings (P0609R3) #89906

Merged
merged 5 commits into from Apr 28, 2024

Conversation

cor3ntin
Copy link
Contributor

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p0609r3.pdf

We support this feature in all language mode.
maybe_unused applied to a binding makes the whole declaration unused. There maybe something more clever to do here but that can be explored as a separate PR.

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p0609r3.pdf

We support this feature in all language mode.
maybe_unused applied to a binding makes the whole declaration unused.
There maybe something more clever to do here but that can be explored
as a separate PR.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 24, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 24, 2024

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p0609r3.pdf

We support this feature in all language mode.
maybe_unused applied to a binding makes the whole declaration unused. There maybe something more clever to do here but that can be explored as a separate PR.


Full diff: https://github.com/llvm/llvm-project/pull/89906.diff

15 Files Affected:

  • (modified) clang/docs/LanguageExtensions.rst (+1)
  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/Basic/Attr.td (+1-1)
  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+10)
  • (modified) clang/include/clang/Sema/DeclSpec.h (+6-4)
  • (modified) clang/include/clang/Sema/ParsedAttr.h (+1)
  • (modified) clang/lib/Frontend/InitPreprocessor.cpp (+1-1)
  • (modified) clang/lib/Parse/ParseDecl.cpp (+31-10)
  • (modified) clang/lib/Sema/DeclSpec.cpp (+2-2)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+2)
  • (modified) clang/test/Lexer/cxx-features.cpp (+1-1)
  • (modified) clang/test/Parser/cxx1z-decomposition.cpp (+30-8)
  • (modified) clang/test/SemaCXX/unused.cpp (+11-1)
  • (modified) clang/www/cxx_status.html (+1-1)
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 84fc4dee02fa80..49afd6c7f06bdd 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1493,6 +1493,7 @@ Conditional ``explicit``                     __cpp_conditional_explicit       C+
 ``if consteval``                             __cpp_if_consteval               C++23         C++20
 ``static operator()``                        __cpp_static_call_operator       C++23         C++03
 Attributes on Lambda-Expressions                                              C++23         C++11
+Attributes on Structured Bindings            __cpp_structured_bindings        C++26         C++03
 ``= delete ("should have a reason");``       __cpp_deleted_function           C++26         C++03
 -------------------------------------------- -------------------------------- ------------- -------------
 Designated initializers (N494)                                                C99           C89
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 64526ed6d06f55..8c8ded8cde0837 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -131,6 +131,8 @@ C++2c Feature Support
 
 - Implemented `P2573R2: = delete("should have a reason"); <https://wg21.link/P2573R2>`_
 
+- Implemented `P0609R3: Attributes for Structured Bindings <https://wg21.link/P0609R3>`_
+
 
 Resolutions to C++ Defect Reports
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 4408d517e70e58..97e06fe7d2e6aa 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3211,7 +3211,7 @@ def ObjCRequiresPropertyDefs : InheritableAttr {
 def Unused : InheritableAttr {
   let Spellings = [CXX11<"", "maybe_unused", 201603>, GCC<"unused">,
                    C23<"", "maybe_unused", 202106>];
-  let Subjects = SubjectList<[Var, ObjCIvar, Type, Enum, EnumConstant, Label,
+  let Subjects = SubjectList<[Var, Binding, ObjCIvar, Type, Enum, EnumConstant, Label,
                               Field, ObjCMethod, FunctionLike]>;
   let Documentation = [WarnMaybeUnusedDocs];
 }
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 38174cf3549f14..74ce0f8b53a48c 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1056,11 +1056,21 @@ def ext_decl_attrs_on_lambda : ExtWarn<
 def ext_lambda_missing_parens : ExtWarn<
   "lambda without a parameter clause is a C++23 extension">,
   InGroup<CXX23>;
+
 def warn_cxx20_compat_decl_attrs_on_lambda : Warning<
   "%select{an attribute specifier sequence|%1}0 in this position "
   "is incompatible with C++ standards before C++23">,
   InGroup<CXXPre23Compat>, DefaultIgnore;
 
+def ext_decl_attrs_on_binding : ExtWarn<
+  "an attribute specifier sequence attached to a structured binding declaration "
+  "is a C++2c extension">, InGroup<CXX26>;
+
+def warn_cxx23_compat_decl_attrs_on_binding : Warning<
+  "an attribute specifier sequence attached to a structured binding declaration "
+  "is incompatible with C++ standards before C++2c">,
+  InGroup<CXXPre26Compat>, DefaultIgnore;
+
 // C++17 lambda expressions
 def err_expected_star_this_capture : Error<
   "expected 'this' following '*' in lambda capture list">;
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index c9eecdafe62c7c..760c7980be52bd 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -36,6 +36,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
+#include <optional>
 
 namespace clang {
   class ASTContext;
@@ -1790,6 +1791,7 @@ class DecompositionDeclarator {
   struct Binding {
     IdentifierInfo *Name;
     SourceLocation NameLoc;
+    std::optional<ParsedAttributes> Attrs;
   };
 
 private:
@@ -2339,10 +2341,10 @@ class Declarator {
   }
 
   /// Set the decomposition bindings for this declarator.
-  void
-  setDecompositionBindings(SourceLocation LSquareLoc,
-                           ArrayRef<DecompositionDeclarator::Binding> Bindings,
-                           SourceLocation RSquareLoc);
+  void setDecompositionBindings(
+      SourceLocation LSquareLoc,
+      MutableArrayRef<DecompositionDeclarator::Binding> Bindings,
+      SourceLocation RSquareLoc);
 
   /// AddTypeInfo - Add a chunk to this declarator. Also extend the range to
   /// EndLoc, which should be the last token of the chunk.
diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h
index 25a5fa05b21c7d..8368d9ce61466d 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -948,6 +948,7 @@ class ParsedAttributes : public ParsedAttributesView {
   ParsedAttributes(AttributeFactory &factory) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
   ParsedAttributes &operator=(const ParsedAttributes &) = delete;
+  ParsedAttributes(ParsedAttributes &&G) = default;
 
   AttributePool &getPool() const { return pool; }
 
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 4f44c3b7b89d4d..fb6435a30cc656 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -704,7 +704,7 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
     Builder.defineMacro("__cpp_nested_namespace_definitions", "201411L");
     Builder.defineMacro("__cpp_variadic_using", "201611L");
     Builder.defineMacro("__cpp_aggregate_bases", "201603L");
-    Builder.defineMacro("__cpp_structured_bindings", "201606L");
+    Builder.defineMacro("__cpp_structured_bindings", "202403L");
     Builder.defineMacro("__cpp_nontype_template_args",
                         "201411L"); // (not latest)
     Builder.defineMacro("__cpp_fold_expressions", "201603L");
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 05ad5ecbfaa0cf..a7846e102a43c7 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -7038,18 +7038,23 @@ void Parser::ParseDirectDeclarator(Declarator &D) {
 void Parser::ParseDecompositionDeclarator(Declarator &D) {
   assert(Tok.is(tok::l_square));
 
+  TentativeParsingAction PA(*this);
+  BalancedDelimiterTracker T(*this, tok::l_square);
+  T.consumeOpen();
+
+  if (isCXX11AttributeSpecifier())
+    DiagnoseAndSkipCXX11Attributes();
+
   // If this doesn't look like a structured binding, maybe it's a misplaced
   // array declarator.
-  // FIXME: Consume the l_square first so we don't need extra lookahead for
-  // this.
-  if (!(NextToken().is(tok::identifier) &&
-        GetLookAheadToken(2).isOneOf(tok::comma, tok::r_square)) &&
-      !(NextToken().is(tok::r_square) &&
-        GetLookAheadToken(2).isOneOf(tok::equal, tok::l_brace)))
+  if (!(Tok.is(tok::identifier) &&
+        NextToken().isOneOf(tok::comma, tok::r_square, tok::kw_alignas,
+                            tok::l_square)) &&
+      !(Tok.is(tok::r_square) &&
+        NextToken().isOneOf(tok::equal, tok::l_brace))) {
+    PA.Revert();
     return ParseMisplacedBracketDeclarator(D);
-
-  BalancedDelimiterTracker T(*this, tok::l_square);
-  T.consumeOpen();
+  }
 
   SmallVector<DecompositionDeclarator::Binding, 32> Bindings;
   while (Tok.isNot(tok::r_square)) {
@@ -7074,13 +7079,27 @@ void Parser::ParseDecompositionDeclarator(Declarator &D) {
       }
     }
 
+    if (isCXX11AttributeSpecifier())
+      DiagnoseAndSkipCXX11Attributes();
+
     if (Tok.isNot(tok::identifier)) {
       Diag(Tok, diag::err_expected) << tok::identifier;
       break;
     }
 
-    Bindings.push_back({Tok.getIdentifierInfo(), Tok.getLocation()});
+    IdentifierInfo *II = Tok.getIdentifierInfo();
+    SourceLocation Loc = Tok.getLocation();
     ConsumeToken();
+
+    ParsedAttributes Attrs(AttrFactory);
+    if (isCXX11AttributeSpecifier()) {
+      Diag(Tok, getLangOpts().CPlusPlus26
+                    ? diag::warn_cxx23_compat_decl_attrs_on_binding
+                    : diag::ext_decl_attrs_on_binding);
+      MaybeParseCXX11Attributes(Attrs);
+    }
+
+    Bindings.push_back({II, Loc, std::move(Attrs)});
   }
 
   if (Tok.isNot(tok::r_square))
@@ -7095,6 +7114,8 @@ void Parser::ParseDecompositionDeclarator(Declarator &D) {
     T.consumeClose();
   }
 
+  PA.Commit();
+
   return D.setDecompositionBindings(T.getOpenLocation(), Bindings,
                                     T.getCloseLocation());
 }
diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index b79683bb32a69e..5f63c857c43067 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -293,7 +293,7 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
 
 void Declarator::setDecompositionBindings(
     SourceLocation LSquareLoc,
-    ArrayRef<DecompositionDeclarator::Binding> Bindings,
+    MutableArrayRef<DecompositionDeclarator::Binding> Bindings,
     SourceLocation RSquareLoc) {
   assert(!hasName() && "declarator given multiple names!");
 
@@ -317,7 +317,7 @@ void Declarator::setDecompositionBindings(
           new DecompositionDeclarator::Binding[Bindings.size()];
       BindingGroup.DeleteBindings = true;
     }
-    std::uninitialized_copy(Bindings.begin(), Bindings.end(),
+    std::uninitialized_move(Bindings.begin(), Bindings.end(),
                             BindingGroup.Bindings);
   }
 }
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 452e00fa32b102..2e27e031b8fabd 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1974,7 +1974,7 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions &LangOpts,
     // it is, by the bindings' expressions).
     bool IsAllPlaceholders = true;
     for (const auto *BD : DD->bindings()) {
-      if (BD->isReferenced())
+      if (BD->isReferenced() || BD->hasAttr<UnusedAttr>())
         return false;
       IsAllPlaceholders = IsAllPlaceholders && BD->isPlaceholderVar(LangOpts);
     }
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index abdbc9d8830c03..1a71a37c0732aa 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -910,6 +910,8 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D,
 
     auto *BD = BindingDecl::Create(Context, DC, B.NameLoc, VarName);
 
+    ProcessDeclAttributeList(S, BD, *B.Attrs);
+
     // Find the shadowed declaration before filtering for scope.
     NamedDecl *ShadowedDecl = D.getCXXScopeSpec().isEmpty()
                                   ? getShadowedDeclaration(BD, Previous)
diff --git a/clang/test/Lexer/cxx-features.cpp b/clang/test/Lexer/cxx-features.cpp
index baaa9d4434e9b7..4a08eb61cd3978 100644
--- a/clang/test/Lexer/cxx-features.cpp
+++ b/clang/test/Lexer/cxx-features.cpp
@@ -222,7 +222,7 @@
 #error "wrong value for __cpp_aggregate_bases"
 #endif
 
-#if check(structured_bindings, 0, 0, 0, 201606, 201606, 201606, 201606)
+#if check(structured_bindings, 0, 0, 0, 202403L, 202403L, 202403L, 202403L)
 #error "wrong value for __cpp_structured_bindings"
 #endif
 
diff --git a/clang/test/Parser/cxx1z-decomposition.cpp b/clang/test/Parser/cxx1z-decomposition.cpp
index 90d60df2e47fe8..81ad13e2381bc1 100644
--- a/clang/test/Parser/cxx1z-decomposition.cpp
+++ b/clang/test/Parser/cxx1z-decomposition.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -std=c++17 %s -verify=expected,cxx17 -fcxx-exceptions
-// RUN: %clang_cc1 -std=c++2b %s -verify=expected,cxx2b -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++17 %s -verify=expected,cxx17,pre2c -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++2b %s -verify=expected,cxx2b,pre2c,post2b -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++2c %s -verify=expected,cxx2c,post2b -fcxx-exceptions
 // RUN: not %clang_cc1 -std=c++17 %s -emit-llvm-only -fcxx-exceptions
 
 struct S { int a, b, c; };
@@ -58,7 +59,7 @@ namespace OtherDecl {
 namespace GoodSpecifiers {
   void f() {
     int n[1];
-    const volatile auto &[a] = n; // cxx2b-warning {{volatile qualifier in structured binding declaration is deprecated}}
+    const volatile auto &[a] = n; // post2b-warning {{volatile qualifier in structured binding declaration is deprecated}}
   }
 }
 
@@ -97,8 +98,8 @@ namespace BadSpecifiers {
     S [a] = s; // expected-error {{cannot be declared with type 'S'}}
     decltype(auto) [b] = s; // expected-error {{cannot be declared with type 'decltype(auto)'}}
     auto ([c2]) = s; // cxx17-error {{decomposition declaration cannot be declared with parenthese}} \
-                     // cxx2b-error {{use of undeclared identifier 'c2'}} \
-                     // cxx2b-error {{expected body of lambda expression}} \
+                     // post2b-error {{use of undeclared identifier 'c2'}} \
+                     // post2b-error {{expected body of lambda expression}} \
 
     // FIXME: This error is not very good.
     auto [d]() = s; // expected-error {{expected ';'}} expected-error {{expected expression}}
@@ -119,9 +120,6 @@ namespace BadSpecifiers {
     [[]] auto [ok_3] = s;
     alignas(S) auto [ok_4] = s;
 
-    // ... but not after the identifier or declarator.
-    // FIXME: These errors are not very good.
-    auto [bad_attr_1 [[]]] = s; // expected-error {{attribute list cannot appear here}} expected-error 2{{}}
     auto [bad_attr_2] [[]] = s; // expected-error {{expected ';'}} expected-error {{}}
   }
 }
@@ -156,3 +154,27 @@ namespace Init {
     S [goodish4] { 4 }; // expected-error {{cannot be declared with type 'S'}}
   }
 }
+
+
+namespace attributes {
+
+struct S{
+    int a;
+    int b = 0;
+};
+
+void err() {
+    auto [[]] = S{0}; // expected-error {{expected unqualified-id}}
+    auto [ alignas(42) a, foo ] = S{0}; // expected-error {{an attribute list cannot appear here}}
+    auto [ c, [[]] d ] = S{0}; // expected-error {{an attribute list cannot appear here}}
+    auto [ e, alignas(42) f ] = S{0}; // expected-error {{an attribute list cannot appear here}}
+}
+
+void ok() {
+    auto [ a alignas(42) [[]], b alignas(42) [[]]] = S{0}; // expected-error 2{{'alignas' attribute only applies to variables, data members and tag types}} \
+                                                           // pre2c-warning  2{{an attribute specifier sequence attached to a structured binding declaration is a C++2c extension}}
+    auto [ c [[]] alignas(42), d [[]] alignas(42) [[]]] = S{0}; // expected-error 2{{'alignas' attribute only applies to variables, data members and tag types}} \
+                                                                // pre2c-warning  2{{an attribute specifier sequence attached to a structured binding declaration is a C++2c extension}}
+}
+
+}
diff --git a/clang/test/SemaCXX/unused.cpp b/clang/test/SemaCXX/unused.cpp
index 0af9e5b68b00df..1f40c1b1ca903d 100644
--- a/clang/test/SemaCXX/unused.cpp
+++ b/clang/test/SemaCXX/unused.cpp
@@ -102,11 +102,21 @@ namespace PR33839 {
     for (auto [x] : a) { // expected-warning {{unused variable '[x]'}}
     }
   }
-  void use() { 
+  void use() {
     f<int>(); // expected-note {{instantiation of}}
     g<true>();
     g<false>();
     h<int>(); // expected-note {{instantiation of}}
   }
 }
+
+namespace maybe_unused_binding {
+
+void test() {
+  struct X { int a, b; } x;
+  auto [a [[maybe_unused]], b] = x; // expected-warning {{an attribute specifier sequence attached to a structured binding declaration is a C++2c extension}}
+}
+
+}
+
 #endif
diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index c233171e63c811..0d796597d05c0e 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -187,7 +187,7 @@ <h2 id="cxx26">C++2c implementation status</h2>
  <tr>
   <td>Trivial infinite loops are not Undefined Behavior</td>
   <td><a href="https://wg21.link/P2809R3">P2809R3</a> (<a href="#dr">DR</a>)</td>
-  <td class="none" align="center">No</td>
+  <td class="unreleased" align="center">Clang 19</td>
  </tr>
  <tr>
   <td>Erroneous behaviour for uninitialized reads</td>

@@ -317,7 +317,7 @@ void Declarator::setDecompositionBindings(
new DecompositionDeclarator::Binding[Bindings.size()];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I'm not super fan of the hidden vector implementation here.
It forces Binding to be default constructible.

@@ -1056,11 +1056,21 @@ def ext_decl_attrs_on_lambda : ExtWarn<
def ext_lambda_missing_parens : ExtWarn<
"lambda without a parameter clause is a C++23 extension">,
InGroup<CXX23>;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a whitespace change.

@yronglin
Copy link
Contributor

yronglin commented Apr 24, 2024

Thank you working on this. I'm really like this feature! I've a question, do we have any further plans to support GNU extension attributes(e.g. __attribute__((aligned)))? Although it is not included in the paper.

@cor3ntin
Copy link
Contributor Author

Thank you working on this. I'm really like this feature! I've a question, do we have any further plans to support GNU extension attributes(e.g. attribute((aligned)))? Although it is not included in the paper.

No, sorry!

My goal here is to increase conformance, not to invent new extensions.
If someone wants to do that work, they would have to motivate the change, do the design leg work and synchronize with GCC.
More generally, supporting GNU syntax in relatively new, C++ specific constructs is probably of limited use.
If there were attributes useful in structured binding that don't yet have a [[]] spelling, we should probably would want to fix that instead.

@erichkeane
Copy link
Collaborator

Thank you working on this. I'm really like this feature! I've a question, do we have any further plans to support GNU extension attributes(e.g. attribute((aligned)))? Although it is not included in the paper.

No, sorry!

My goal here is to increase conformance, not to invent new extensions. If someone wants to do that work, they would have to motivate the change, do the design leg work and synchronize with GCC. More generally, supporting GNU syntax in relatively new, C++ specific constructs is probably of limited use. If there were attributes useful in structured binding that don't yet have a [[]] spelling, we should probably would want to fix that instead.

Agreed 100%.


void test() {
struct X { int a, b; } x;
auto [a [[maybe_unused]], b] = x; // expected-warning {{an attribute specifier sequence attached to a structured binding declaration is a C++2c extension}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any standard attributes other than this that make sense on SB's? If not, I'd like all of the standards ones tested to show what the behavior is (and 'not valid here' type errors are totally acceptable).

[[indeterminate]] seems useful, but the rest should likely result in a rejection.

Additionally, we should do an audit of ALL our "C++" spelling attributes to see which make sense here and to make sure they are treated reasonably. I'm not asking to do that HERE, but a bug in our bug tracker (perhaps with a 'good starter bug' tag, particularly if we list ALL our attributes that need auditing) would be acceptable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any standard attributes other than this that make sense on SB's? If not, I'd like all of the standards ones tested to show what the behavior is (and 'not valid here' type errors are totally acceptable).

No, I can add tests

[[indeterminate]] seems useful, but the rest should likely result in a rejection.

We do not have this one

Additionally, we should do an audit of ALL our "C++" spelling attributes to see which make sense here and to make sure they are treated reasonably. I'm not asking to do that HERE, but a bug in our bug tracker (perhaps with a 'good starter bug' tag, particularly if we list ALL our attributes that need auditing) would be acceptable.

Yes, I can create an issue once we land that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done
I created an issue to allow the deprecated attribute cplusplus/CWG#530
(We now support that as an extension , and it would be challenging not to)


void test() {
struct X { int a, b; } x;
auto [a [[maybe_unused]], b] = x; // expected-warning {{an attribute specifier sequence attached to a structured binding declaration is a C++2c extension}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a test to check the [[maybe_unused]] takes effect.
e.g. checks whether there has warning: unused variable '[a, b]' [-Wunused-variable].

I've a question about this test case. We only add [[maybe_unused]] to a , IIUC, should we also emit a warning that unused variable 'b'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is that test.
By marking one of the member unused, we don't warn on any of the member.
This perhaps not perfect but the standard already specified that if one of the member is used, we should not warn on any of the biding

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree!

Copy link
Contributor

@yronglin yronglin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cor3ntin cor3ntin merged commit 6dd9061 into llvm:main Apr 28, 2024
5 checks passed
@@ -1115,7 +1115,9 @@ int64_t Decl::getID() const {

const FunctionType *Decl::getFunctionType(bool BlocksToo) const {
QualType Ty;
if (const auto *D = dyn_cast<ValueDecl>(this))
if (const auto *D = dyn_cast<BindingDecl>(this))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems causes the warning. [-Wunused-but-set-variable]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix that shortly

Comment on lines 188 to +190
<td>Trivial infinite loops are not Undefined Behavior</td>
<td><a href="https://wg21.link/P2809R3">P2809R3</a> (<a href="#dr">DR</a>)</td>
<td class="none" align="center">No</td>
<td class="unreleased" align="center">Clang 19</td>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the wrong paper was marked as implemented

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix that shortly

@kstoimenov
Copy link
Contributor

This is breaking a sanitizer build bot because it is causing a memory leak: https://lab.llvm.org/buildbot/#/builders/168/builds/20146.

@cor3ntin could you please take a look?

Thanks!

@cor3ntin
Copy link
Contributor Author

@kstoimenov Looking now

cor3ntin added a commit to cor3ntin/llvm-project that referenced this pull request Apr 29, 2024
* Fix a leak
* Fix a maybe unused warning
* Fix incorrect cxx_status entry
@cor3ntin
Copy link
Contributor Author

#90495

cor3ntin added a commit to cor3ntin/llvm-project that referenced this pull request Apr 29, 2024
* Fix a leak
* Fix a maybe unused warning
* Fix incorrect cxx_status entry
cor3ntin added a commit that referenced this pull request Apr 29, 2024
* Fix a leak
* Fix a maybe unused warning
* Fix incorrect cxx_status entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants