aaron.ballman added a comment.
This is awesome, thank you for working on it! @to268 and I had a really good
discussion during my office hours and what we decided for next steps were:
0) Next time you upload a patch, please use `-U9999` to give the patch more
context for reviewers to see.
1. Need to handle function pointers: `auto (*)(void)` and `void (*)(auto)`
should both be rejected - you may want to consider adding logic to
`Sema::BuildPointerType()` to semantically restrict forming such a function
pointer type.
2. Need a ton of random tests:
auto file_scope_1 = 12;
// This <might> be something we accept because the redundant storage class
specifier is not
// specifying automatic storage duration due to the lack of a type specifier,
or we might reject
// because of the redundant storage class specifier. I have a question out to
the author to clarify.
auto auto file_scope_2 = 12;
void func(void) {
int i;
_Generic(i, auto : 0); // Should reject auto in an association
_Alignof(auto); // Should also reject
_Alignas(auto); // Should also reject
auto j = ({ auto x = 12; x; }); // Should work, x and j should both be int
auto auto k = 12; // 99% this is intended to be accepted; first `auto` is
the storage class specifier, second `auto` is a redundant storage class
specifier
const int ci = 12;
auto yup = ci;
yup = 12; // Okay, the type of yup is int, not const int
_Atomic(auto) atom1 = 12; // Should reject, asking WG14 about it
_Atomic auto atom2 = 12; // Should also reject
}
`
It's worth noting that `auto` in C is a very, very strange beast. It's a
storage-class-specifier, not a type-name. If there's a storage class specifier
and NO TYPE NAME, then the type is inferred. So grammar productions that use
type-name without also using `storage-class-specifier` should reject any use of
`auto` because that's not a valid type name.
================
Comment at: clang/test/C/C2X/n3007.c:6
+ */
+#include <stdalign.h>
+
----------------
Rather than include the system header, you should use `_Alignas` and `_Alignof`
spellings.
================
Comment at: clang/test/C/C2X/n3007.c:45
+ struct B { auto b; }; // expected-error {{'auto' not allowed in struct
member}}
+ typedef auto auto_type; // expected-error {{'auto' not allowed in typedef}}
+}
----------------
Heh, this doesn't have much to do with structs. :-)
================
Comment at: clang/test/C/C2X/n3007.c:50
+ auto test_char = 'A';
+ auto test_char_ptr = "test";
+ auto something; // expected-error {{declaration of variable 'something' with
deduced type 'auto' requires an initializer}}
----------------
An additional tests here that's interesting is:
```
_Static_assert(_Generic(test_char_ptr, const char * : 1, char * : 2) == 2, "C
is weird");
```
================
Comment at: clang/test/Parser/c2x-auto.c:9
+ int a;
+ int b;
+ union {
----------------
Should also test a structure member just to have the coverage.
================
Comment at: clang/test/Sema/c2x-auto.c:3
+
+#include <stdalign.h>
+
----------------
Same suggestion here as above.
================
Comment at: clang/test/Sema/c2x-auto.c:11-20
+void test_structs(void) {
+ struct s_auto { auto a; }; // expected-error {{'auto' not allowed in
struct member}}
+ auto s_int = (struct { int a; } *)0;
+ typedef auto auto_type; // expected-error {{'auto' not allowed in
typedef}}
+}
+
+void test_sizeof_alignas(void) {
----------------
This seems to be duplicated from the parser tests -- there is only a need to
test this once (and in each of these cases it seems to be a parsing
restriction, so the tests should live in Parser).
================
Comment at: clang/test/Sema/c2x-auto.c:32
+ auto double_cl = (double){2.5};
+ auto auto_cl = (auto){13}; // expected-error {{expected expression}}
+ auto array[] = { 1, 2, 3 }; // expected-error {{'array' declared as array of
'auto'}}
----------------
This should have a FIXME comment -- we should produce a better diagnostic here.
That we give an error is correct though!
C2x 6.5.2.5p4 specifies that compound literal is rewritten as if it were a
declaration: `SC typeof(T) ID = { IL };` (where SC is the storage class, T is
the type, ID is a program-wide unique ID, and IL is the initializer list).
C2x 6.7.2.5p1 only allows `typeof` on an `expression` or a `type-name`, and
`type-name` does not include `auto` as an option.
So the rewrite makes this invalid.
================
Comment at: clang/www/c_status.html:1196
<td><a
href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3007.htm";>N3007</a></td>
- <td class="none" align="center">No</td>
+ <td class="full" align="center">Yes</td>
</tr>
----------------
This should use class `unreleased` and specify `Clang 16` instead of `Yes`
("Yes" basically means "we have no idea when we started supporting something,
we've probably supported this forever")
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133289/new/
https://reviews.llvm.org/D133289
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
