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] Add test for CWG2149 "Brace elision and array length deduction" #90079

Merged
merged 1 commit into from Apr 26, 2024

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Apr 25, 2024

This patch adds test for CWG2149, following P3106R1 "Clarifying rules for brace elision in aggregate initialization" and a clarification note on top of it added on April 2024.

I haven't found a better way to check for equality of values inside array in 98 mode than to dump AST. I'm open to suggestions there.

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Apr 25, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 25, 2024

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

This patch adds test for CWG2149, following P3106R1 "Clarifying rules for brace elision in aggregate initialization" and a clarification note on top of it added on April 2024.

I haven't found a better way to check for equality of values inside array in 98 mode than to dump AST. I'm open to suggestions there.


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

2 Files Affected:

  • (added) clang/test/CXX/drs/cwg2149.cpp (+77)
  • (modified) clang/www/cxx_dr_status.html (+1-1)
diff --git a/clang/test/CXX/drs/cwg2149.cpp b/clang/test/CXX/drs/cwg2149.cpp
new file mode 100644
index 00000000000000..d0f8cb2dfc0a93
--- /dev/null
+++ b/clang/test/CXX/drs/cwg2149.cpp
@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify=expected,cxx98 -fexceptions -fcxx-exceptions -pedantic-errors -ast-dump | FileCheck %s --check-prefixes CXX98
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++23 -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++2c -triple x86_64-unknown-unknown %s -verify=expected,since-cxx11 -fexceptions -fcxx-exceptions -pedantic-errors
+
+#if __cplusplus == 199711L
+#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
+// cxx98-error@-1 {{variadic macros are a C99 feature}}
+#endif
+
+namespace cwg2149 { // cwg2149: 3.1 drafting 2024-04
+#if __cplusplus <= 201103L
+struct X { int i, j, k; };
+#else
+struct X { int i, j, k = 42; };
+#endif
+
+template<int N> 
+void f1(const X(&)[N]); // #cwg2149-f1
+
+template<int N>
+void f2(const X(&)[N][2]); // #cwg2149-f2
+
+void f() {
+  X a[] = { 1, 2, 3, 4, 5, 6 };
+  static_assert(sizeof(a) / sizeof(X) == 2, "");
+  X b[2] = { { 1, 2, 3 }, { 4, 5, 6 } };
+  X c[][2] = { 1, 2, 3, 4, 5, 6 };
+  static_assert(sizeof(c) / sizeof(X[2]) == 1, "");
+  
+  #if __cplusplus >= 201103L
+  constexpr X ca[] = { 1, 2, 3, 4, 5, 6 };
+  constexpr X cb[2] = { { 1, 2, 3 }, { 4, 5, 6 } };
+  static_assert(ca[0].i == cb[0].i, "");
+  static_assert(ca[0].j == cb[0].j, "");
+  static_assert(ca[0].k == cb[0].k, "");
+  static_assert(ca[1].i == cb[1].i, "");
+  static_assert(ca[1].j == cb[1].j, "");
+  static_assert(ca[1].k == cb[1].k, "");
+
+  f1({ 1, 2, 3, 4, 5, 6 });
+  // since-cxx11-error@-1 {{no matching function for call to 'f1'}}
+  //   since-cxx11-note@#cwg2149-f1 {{candidate function [with N = 6] not viable: no known conversion from 'int' to 'const X' for 1st argument}}
+  f2({ 1, 2, 3, 4, 5, 6 });
+  // since-cxx11-error@-1 {{no matching function for call to 'f2'}}
+  //   since-cxx11-note@#cwg2149-f2 {{candidate function [with N = 6] not viable: no known conversion from 'int' to 'const X[2]' for 1st argument}}
+  #endif
+}
+} // namespace cwg2149
+
+// Constant evaluation is not powerful enough in 98 mode to check for equality
+// via static_assert, even with constant folding enabled.
+
+// CXX98:       VarDecl {{.+}} a 'X[2]'
+// CXX98-NEXT:  `-InitListExpr {{.+}} 'X[2]'
+// CXX98-NEXT:    |-InitListExpr {{.+}} 'X':'cwg2149::X'
+// CXX98-NEXT:    | |-IntegerLiteral {{.+}} 'int' 1
+// CXX98-NEXT:    | |-IntegerLiteral {{.+}} 'int' 2
+// CXX98-NEXT:    | `-IntegerLiteral {{.+}} 'int' 3
+// CXX98-NEXT:    `-InitListExpr {{.+}} 'X':'cwg2149::X'
+// CXX98-NEXT:      |-IntegerLiteral {{.+}} 'int' 4
+// CXX98-NEXT:      |-IntegerLiteral {{.+}} 'int' 5
+// CXX98-NEXT:      `-IntegerLiteral {{.+}} 'int' 6
+
+// CXX98:       VarDecl {{.+}} b 'X[2]'
+// CXX98-NEXT:  `-InitListExpr {{.+}} 'X[2]'
+// CXX98-NEXT:    |-InitListExpr {{.+}} 'X':'cwg2149::X'
+// CXX98-NEXT:    | |-IntegerLiteral {{.+}} 'int' 1
+// CXX98-NEXT:    | |-IntegerLiteral {{.+}} 'int' 2
+// CXX98-NEXT:    | `-IntegerLiteral {{.+}} 'int' 3
+// CXX98-NEXT:    `-InitListExpr {{.+}} 'X':'cwg2149::X'
+// CXX98-NEXT:      |-IntegerLiteral {{.+}} 'int' 4
+// CXX98-NEXT:      |-IntegerLiteral {{.+}} 'int' 5
+// CXX98-NEXT:      `-IntegerLiteral {{.+}} 'int' 6
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 83b71e7c122d1e..ea8872c91be604 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -12702,7 +12702,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2149.html">2149</a></td>
     <td>drafting</td>
     <td>Brace elision and array length deduction</td>
-    <td align="center">Not resolved</td>
+    <td title="Clang 3.1 implements 2024-04 resolution" align="center">Not Resolved*</td>
   </tr>
   <tr id="2150">
     <td><a href="https://cplusplus.github.io/CWG/issues/2150.html">2150</a></td>

@Endilll
Copy link
Contributor Author

Endilll commented Apr 25, 2024

Status of 2149 changed just yesterday to say DR per 2024 Tokyo straw poll. That's where the discrepancy between official cwg_index.html and CWG GitHub repo comes (that I link in the description).

@cor3ntin
Copy link
Contributor

Can you add examples 14, 16, 17, 18 of the paper?
I'd be if they are only tested in C++11+ modes
Then we can mark the paper as implemented (bonus point for finding the oldest conforming version)

@Endilll
Copy link
Contributor Author

Endilll commented Apr 25, 2024

Can you add examples 14, 16, 17, 18 of the paper?

I'll add them to conformance test suite in a subsequent PR. They don't belong here, as the issue is about deducing array length from brace initializer, but the examples you're listing are focusing on various aspects of how initializer-clauses appertain to aggregate elements.

I can, though, derive more tests from them that check for deduced array length in various appertainment cases, if you feel current coverage of the issue is not sufficient.

Copy link
Contributor

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

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

Two questions, and more tests from the paper would probably be nice, as Corentin already pointed out, but what’s already here lgtm.

clang/www/cxx_dr_status.html Show resolved Hide resolved
clang/test/CXX/drs/cwg2149.cpp Show resolved Hide resolved
@Sirraide
Copy link
Contributor

and more tests from the paper would probably be nice

Actually, yeah, now that I think about it the main question that the DR addresses is already being tested, so it’s probably fine the way it is atm.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

I'm happy to approve that as is, I expect additional tests to materialize for the rest of the paper.
I'm not concerned about the status page, a new core issue list should materialize any day now

Thanks

@Endilll Endilll merged commit 1728a56 into llvm:main Apr 26, 2024
8 checks passed
@Endilll Endilll deleted the cwg2149 branch April 26, 2024 14:34
Endilll added a commit that referenced this pull request Apr 29, 2024
Currently we're using official publication of CWG issue list available
at https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html.
Unfortunately, it's not updated as frequently as we sometimes need. For
instance, recently there was a confusion during review of CWG2149 test
(#90079 (comment)).
This patch changes our `make_cxx_dr_status` script to use issue list
from CWG GitHub repository. I confirmed with CWG chair that this is the
most up-to-date source of information on CWG issues.

Changing the source of issue list uncovered previously unhandled
"tentatively ready" status of an issue. This status is considered
unresolved by the script (like `open`, `drafting`, and `review`), as the
resolution might change during face-to-face CWG meeting.

I also noticed that CWG decided to handle 2561 differently from what
we're doing, so this DR is now considered not available in Clang,
despite being declared as available since 18. CC @cor3ntin.

This patch also brings new issues into our DR list, so if someone was
waiting for a newly-filed issue to appear on our status page, it should
appear with this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ clang Clang issues not falling into any other category test-suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants