[PATCH] D29401: Fix MSVC Compatibility around dependent type with missing 'typename'

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: lib/Parse/ParseStmt.cpp:186
     // found.
-    if (Next.isNot(tok::coloncolon)) {
+    if (Next.isNot(tok::coloncolon) && (!getLangOpts().MSVCCompat ||
+        Next.isNot(tok::less))) {
----------------
efriedma wrote:
> erichkeane wrote:
> > efriedma wrote:
> > > erichkeane wrote:
> > > > Clang-tidy created this layout here that I'm not thrilled with, if OK, I'd like to move the entirety of the 2nd component to the "&&" on its own line.  Additionally, if anyone has a better way to do this logic, I'm all ears!
> > > Why is this checking for MSVCCompat?  I think we want to detect constructs like your testcase in all modes so we can generate a good error message.
> > We get a good error message here ("typename missing") in normal mode.  The issue here is that the examples below work in MSVC's relaxed 'typename' situation, thus this should only be accepting code in MSVC mode, right?  Or am I missing something.
> This is what I see for the testcase in your commit message on trunk:
> 
> ```
> <stdin>:7:4: error: expected ';' after expression
>   S<TMP>::TD varname =0;
>    ^
>    ;
> <stdin>:7:14: error: use of undeclared identifier 'varname'
>   S<TMP>::TD varname =0;
>              ^
> <stdin>:7:3: error: missing 'typename' prior to dependent type name 'S<int>::TD'
>   S<TMP>::TD varname =0;
>   ^~~~~~~~~~
> <stdin>:11:3: note: in instantiation of function template specialization 'foo<int>' requested here
>   foo<int>();
>   ^
> 3 errors generated.
> ```
> 
> Technically speaking, we do get the "missing typename" message, but I still wouldn't call this result "a good error message".
Ah, I see!  You're right, I got caught up in this a bunch.  I'll look to see if I can get both cases to be better.  Thanks!


https://reviews.llvm.org/D29401