Add more tests for _Countof #133333
Add more tests for _Countof #133333
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
alejandro-colomar
force-pushed
the
countof_tests
branch
from
c9ce60f to
fe746be
Compare
There was a problem hiding this comment.
Thanks for the additional test coverage! It looks like precommit CI found some issues with your tests that should be addressed: https://buildkite.com/llvm-project/github-pull-requests/builds/162502#0195d9e8-2c29-426e-9be0-afda7efd7d93
alejandro-colomar
force-pushed
the
countof_tests
branch
from
fe746be to
a651447
Compare
alejandro-colomar
force-pushed
the
countof_tests
branch
from
a651447 to
6c5c502
Compare
alejandro-colomar
force-pushed
the
countof_tests
branch
from
6c5c502 to
b8f95e7
Compare
alejandro-colomar
force-pushed
the
countof_tests
branch
2 times, most recently
from
eb63ad4 to
3660f83
Compare
alejandro-colomar
force-pushed
the
countof_tests
branch
2 times, most recently
from
d393955 to
923856a
Compare
Link: <llvm#102836> Link: <llvm#133125> Cc: Aaron Ballman <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar
force-pushed
the
countof_tests
branch
from
923856a to
f4f1f56
Compare
|
@llvm/pr-subscribers-clang Author: Alejandro Colomar (alejandro-colomar) ChangesLink: <#102836> Cc: @AaronBallman I haven't yet tried to run these tests. I've pasted and adapted them from the ones I wrote for my own implementation. (I have a few more, which I'll adapt once I know I adapted these correctly.) Revisions: <details>
</details> <details>
</details> <details>
</details> <details>
</details> <details>
</details> <details>
</details> <details>
</details> <details>
</details> Full diff: https://github.com/llvm/llvm-project/pull/133333.diff 1 Files Affected:
diff --git a/clang/test/C/C2y/n3369.c b/clang/test/C/C2y/n3369.c
index 389828b52b6a2..db26040d8cf44 100644
--- a/clang/test/C/C2y/n3369.c
+++ b/clang/test/C/C2y/n3369.c
@@ -17,7 +17,11 @@
#error "Expected to have _Countof support"
#endif
+#define NULL ((void *) 0)
+
int global_array[12];
+int global_multi_array[12][34];
+int global_num;
void test_parsing_failures() {
(void)_Countof; // expected-error {{expected expression}}
@@ -36,6 +40,12 @@ void test_semantic_failures() {
expected-note {{forward declaration of 'struct S'}}
struct T { int x; };
(void)_Countof(struct T); // expected-error {{'_Countof' requires an argument of array type; 'struct T' invalid}}
+ struct U { int x[3]; };
+ (void)_Countof(struct U); // expected-error {{'_Countof' requires an argument of array type; 'struct U' invalid}}
+ int a[3];
+ (void)_Countof(&a); // expected-error {{'_Countof' requires an argument of array type; 'int (*)[3]' invalid}}
+ int *p;
+ (void)_Countof(p); // expected-error {{'_Countof' requires an argument of array type; 'int *' invalid}}
}
void test_constant_expression_behavior(int n) {
@@ -81,6 +91,22 @@ void test_with_function_param(int array[12], int (*array_ptr)[12], int static_ar
(void)_Countof(static_array); // expected-error {{'_Countof' requires an argument of array type; 'int *' invalid}}
}
+void test_func_fix_fix(int i, char (*a)[3][5], int (*x)[_Countof(*a)], char (*)[_Generic(x, int (*)[3]: 1)]); // expected-note {{passing argument to parameter}}
+void test_func_fix_var(int i, char (*a)[3][i], int (*x)[_Countof(*a)], char (*)[_Generic(x, int (*)[3]: 1)]); // expected-note {{passing argument to parameter}}
+void test_func_fix_uns(int i, char (*a)[3][*], int (*x)[_Countof(*a)], char (*)[_Generic(x, int (*)[3]: 1)]); // expected-note {{passing argument to parameter}}
+
+void test_funcs() {
+ int i3[3];
+ int i5[5];
+ char c35[3][5];
+ test_func_fix_fix(5, &c35, &i3, NULL);
+ test_func_fix_fix(5, &c35, &i5, NULL); // expected-warning {{incompatible pointer types passing 'int (*)[5]' to parameter of type 'int (*)[3]'}}
+ test_func_fix_var(5, &c35, &i3, NULL);
+ test_func_fix_var(5, &c35, &i5, NULL); // expected-warning {{incompatible pointer types passing 'int (*)[5]' to parameter of type 'int (*)[3]'}}
+ test_func_fix_uns(5, &c35, &i3, NULL);
+ test_func_fix_uns(5, &c35, &i5, NULL); // expected-warning {{incompatible pointer types passing 'int (*)[5]' to parameter of type 'int (*)[3]'}}
+}
+
void test_multidimensional_arrays() {
int array[12][7];
static_assert(_Countof(array) == 12);
@@ -102,6 +128,11 @@ void test_unspecified_array_length() {
static_assert(_Countof(**x) == 3);
}
+void test_completed_array() {
+ int a[] = {1, 2, global_num};
+ static_assert(_Countof(a) == 3);
+}
+
// Test that the return type of _Countof is what you'd expect (size_t).
void test_return_type() {
static_assert(_Generic(typeof(_Countof global_array), typeof(sizeof(0)) : 1, default : 0));
@@ -121,10 +152,14 @@ void test_typedefs() {
static_assert(_Countof(*x) == 12);
}
-void test_zero_size_arrays() {
+void test_zero_size_arrays(int n) {
int array[0]; // expected-warning {{zero size arrays are an extension}}
static_assert(_Countof(array) == 0);
static_assert(_Countof(int[0]) == 0); // expected-warning {{zero size arrays are an extension}}
+ int multi_array[0][n]; // FIXME: Should trigger -Wzero-length-array
+ static_assert(_Countof(multi_array) == 0);
+ int another_one[0][3]; // expected-warning {{zero size arrays are an extension}}
+ static_assert(_Countof(another_one) == 0);
}
void test_struct_members() {
@@ -144,3 +179,18 @@ void test_compound_literals() {
static_assert(_Countof((int[2]){}) == 2);
static_assert(_Countof((int[]){1, 2, 3, 4}) == 4);
}
+
+/* We don't get a diagnostic for test_f1(), because it ends up unused
+ * as _Countof() results in an integer constant expression, which is not
+ * evaluated. However, test_f2() ends up being evaluated, since 'a' is
+ * a VLA.
+ */
+static int test_f1();
+static int test_f2(); // FIXME: Should trigger function 'test_f2' has internal linkage but is not defined
+
+void test_symbols() {
+ int a[global_num][global_num];
+
+ static_assert(_Countof(global_multi_array[test_f1()]) == 34);
+ (void)_Countof(a[test_f2()]);
+}
|
|
@AaronBallman This is ready. Still, my patches for GCC had another set of tests that are completely missing here: run-time tests (assert(3)) for non-ICE (so, VLA). Maybe you could add a set of tests that are run (not just compiled). |
We don't do end-to-end testing in Clang directly (those kinds of tests are sometimes more fragile because of differences between host platforms); we usually shunt that sort of thing off into llvm-test-suite, which gets less visibility and has less post-commit bot coverage, so we typically only do that for special cases. Instead we test what LLVM IR we emit and we rely on the backend tests to verify that the given LLVM IR instructions behave as intended. |
|
@alejandro-colomar Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Thanks! Makes sense. |
Link: #102836
Link: #133125
Cc: @AaronBallman
I haven't yet tried to run these tests. I've pasted and adapted them from the ones I wrote for my own implementation. (I have a few more, which I'll adapt once I know I adapted these correctly.)
Revisions:
v2
global_numinstead oferrno.v2b
v2c
v2d
v2e
v3
expected-warninginto a FIXME note (pre-existing false negative).v3b
v3c