[PATCH] D133289: [C2X] N3007 Type inference for object definitions

[Aaron Ballman via Phabricator via cfe-commits]

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