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

[Guillot Tony via Phabricator via cfe-commits]

to268 marked 7 inline comments as done.
to268 added a comment.

This is a status update of the patch.



================
Comment at: clang/lib/Parse/ParseExpr.cpp:1526
+    // This is a temporary fix while we don't support C2x 6.5.2.5p4
+    if (getLangOpts().C2x && GetLookAheadToken(2).getKind() == tok::l_brace) {
+      Diag(Tok, diag::err_c2x_auto_compound_literal_not_allowed);
----------------
aaron.ballman wrote:
> I don't think this is quite right; I suspect we're going to need more complicated logic here. Consider a case like: `(auto const){ 12 }` or `(auto *){ nullptr }` where two tokens ahead is `)` and not `{`. (Note, these type specifications can be pretty arbitrarily complex.)
> 
> Given that this is a temporary workaround for a lack of support for storage class specifiers in compound literals, perhaps another approach is to not try to catch uses in compound literals. Instead, we could add tests with FIXME comments to demonstrate the behavior we get with compound literals now, and when we do that storage class specifier work, we will (hopefully) break those tests and come back to make them correct.
I think it's better to make a test of the actual behavior than trying to correct this case. 


================
Comment at: clang/lib/Parse/ParseExpr.cpp:1530-1531
+    }
+  }
+    [[fallthrough]];
+
----------------
aaron.ballman wrote:
> This looks a bit less visually jarring to me (the indentation differences were distracting). WDYT?
That's better but i remember that `clang-format` has failed on that.


================
Comment at: clang/test/C/C2x/n3007.c:13
+  int* pc = &c; // expected-warning {{incompatible pointer types initializing 'int *' with an expression of type 'unsigned long *'}}
+
+  const int ci = 12;
----------------
aaron.ballman wrote:
> I'd also like a test for:
> ```
> _Atomic auto i = 12;
> _Atomic(auto) j = 12;
> 
> _Atomic(int) foo(void);
> auto k = foo(); // Do we get _Atomic(int) or just int?
> ```
The _Atomic qualifier is dropped so the type of k is an `int`


================
Comment at: clang/test/C/C2x/n3007.c:40-41
+void test_arrary(void) {
+  auto a[4];          // expected-error {{'a' declared as array of 'auto'}}
+  auto b[] = {1, 2};  // expected-error {{'b' declared as array of 'auto'}}
+}
----------------
aaron.ballman wrote:
> I think other array cases we want to test are:
> ```
> auto a = { 1 , 2 }; // Error
> auto b = { 1, }; // OK
> auto c = (int [3]){ 1, 2, 3 }; // OK
> ```
These statements are not working:
```
auto b = { 1, }; // Not valid
auto d = { 1 };  // Not valid
```
I think it's related to the compound literal.



================
Comment at: clang/test/C/C2x/n3007.c:56
+
+  _Static_assert(_Generic(test_char_ptr, const char * : 1, char * : 2) == 2, "C is weird");
+}
----------------
aaron.ballman wrote:
> I'd also like to ensure we reject:
> ```
> typedef auto (*fp)(void);
> typedef void (*fp)(auto);
> 
> _Generic(0, auto : 1);
> ```
> and we should ensure we properly get integer promotions:
> ```
> short s;
> auto a = s;
> _Generic(a, int : 1);
> ```
> and a test for use of an explicit pointer declarator (which is UB that we can define if we want to):
> ```
> int a;
> auto *ptr = &a; // Okay?
> auto *ptr = a; // Error
> ```
```
> and we should ensure we properly get integer promotions:
> ```
> short s;
> auto a = s;
> _Generic(a, int : 1);
> ```
I got the error `controlling expression type 'short' not compatible with any generic association type` so we don't promote from `short` to `int`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133289/new/

https://reviews.llvm.org/D133289