[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Parse/ParseDecl.cpp:3105
+                              isClassTemplateDeductionContext(DSContext),
+                              /*AllowImplicitTypename=*/true);
 
----------------
This looks overly broad: shouldn't this depend on the DSContext?


================
Comment at: lib/Parse/Parser.cpp:1778
+            /*IsClassTemplateDeductionContext*/true,
+            /*AllowImplicitTypename*/false)) {
       SourceLocation BeginLoc = Tok.getLocation();
----------------
Don't we need to sometimes allow an implicit typename here for correct disambiguation?


================
Comment at: test/CXX/temp/temp.res/p5.cpp:1
+// RUN: %clang_cc1 -std=c++2a -pedantic -verify %s
+
----------------
Negative tests are as important as positive tests; please add as many cases as you can think of where this should *not* apply, to make sure it doesn't. Pay particular attention to grammar ambiguities that typename resolves.


================
Comment at: test/CXX/temp/temp.res/p5.cpp:15
+  // it is a qualified name in a type-id-only context (see below), or
+  // [it's smallest enclosing [/new/defining/]-type-id is]:
+  // - a new-type-id
----------------
it's -> its


================
Comment at: test/FixIt/fixit.cpp:214
-  expected-error {{function definition declared 'typedef'}} \
-  expected-error {{missing 'typename' prior to dependent}}
-  return Mystery<T>::get();
----------------
Please retain this, and add a FixItHint to the new extension warning so that it still works.


================
Comment at: test/SemaCXX/PR11358.cpp:15
     void test() {
-      Container::iterator i = c.begin(); // expected-error{{missing 'typename'}}
+      Container::iterator i = c.begin(); // expected-warning{{implicit 'typename' is a C++2a extension}}
     }
----------------
This is a bug. There's no implicit typename in this context. (Likewise later in this file.)


================
Comment at: test/SemaCXX/unknown-type-name.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wc++2a-compat -fsyntax-only -verify -std=c++98 %s
----------------
Several of the changes in this file look wrong to me.


Repository:
  rC Clang

https://reviews.llvm.org/D53847