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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: sammccall, rjmccall, efriedma.
aaron.ballman added a comment.

In D133289#3784117 <https://reviews.llvm.org/D133289#3784117>, @aaron.ballman wrote:

> One thought that occurred to me is that the way C has this specified is awfully effectively the same way implicit int worked. It may be worth exploring a change to our implicit int functionality to instead generate an implicit `auto` type when in C2x mode.

I've been thinking about this more and I'm starting to make myself a bit uncomfortable with the current approach, at least in terms of how we're handling it on the parsing side of things. I think it's reasonable for us to form an `AutoType` when we eventually get down to forming the type. But I'm uncomfortable with not modeling the language when parsing. e.g., I think we want to parse `auto` as a storage class specifier rather than a type specifier, and we want to rely on the lack of a type specifier coupled with use of `auto` as a storage class specifier to determine the situations where we want to infer a type. The resulting semantics should be basically equivalent, but this ensures that we're properly parsing as the language expects which helps us be forwards-compatible with future changes in C that build on top of this being a storage class specifier rather than a type.

Adding a few more reviewers for some extra opinions on this.



================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:374
+def err_c2x_auto_compound_literal_not_allowed: Error<
+  "'auto' is not allowed in compound litterals">;
 def ext_auto_type : Extension<
----------------



================
Comment at: clang/lib/Parse/ParseExpr.cpp:1515
+    // 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);
----------------
Why would this not be handled from `Sema::ActOnCompoundLiteral()`?


================
Comment at: clang/test/C/C2x/n3007.c:7
+void test_qualifiers(int x, const int y) {
+  // TODO: prohibit cont auto
+  const auto a = x;
----------------
Why? My reading of the grammar is that `const auto` is valid. `const` is a type-specifier-qualifier declaration-specifier, and `auto` is a storage-class-specifier declaration-specifier, and a declaration is allowed to use a sequence of declaration-specifiers.


================
Comment at: clang/test/C/C2x/n3007.c:15
+
+  // TODO: prohibit cont auto
+  const int ci = 12;
----------------



================
Comment at: clang/test/Sema/c2x-auto.c:49
+    auto b = 9;
+    auto c = a + b;
+  }
----------------
to268 wrote:
> shafik wrote:
> > When I made the comment about the example from the proposal, this was what I was thinking about.
> Do i need to treat shadowing when using `auto` as invalid?
You shouldn't need to do anything special there, I think. It should naturally fall out that you cannot use a variable in its own initialization. Note, you don't even need an inner scope for that: https://godbolt.org/z/EYa3fMxqx (example is compiled in C++ mode).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289