Fix MSVC Compatibility around dependent type with missing 'typename'

[erichkeane]

Summary

As we know, MSVC is pretty relaxed when it comes to 'typename'. Clang so far does a pretty darn good job of permitting the behavior, however there was 1 missed case that compiles in MSVC (as of 2015), but not in clang. I've updated the test for similar ones that also/still pass, but were not tested.

template<typename T>
struct S { 
  typedef int TD;
};
template<class TMP>
void foo() {
  S<TMP>::TD varname =0;
}

void foo2(){
  foo<int>();
}

The above was previously an error in clang in -fms-compatibility mode. This patch alters the parser in MSVC mode to correctly assume the 'typename' above where necessary.

[erichkeane]

Some quick line-comments here.

lib/Parse/ParseStmt.cpp

186

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!

test/SemaCXX/MicrosoftCompatibility.cpp

222

This is the line that previously failed. Curiously, the one above and below seemed to succeed without this change.

[erichkeane]

Thoughts?

[efriedma]

lib/Parse/ParseStmt.cpp

186

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.

test/SemaCXX/MicrosoftCompatibility.cpp

222

The first one is obviously a declaration because of the "const" keyword, so we don't follow the same codepath. I would guess the last one hits the "Next.isNot(tok::coloncolon)" check in the if statment you're modifying.

A couple more related testcases:

A<T>::TYPE const var3 = 2; // const after type
A<T>::TYPE *var3 = 2; // we can't tell this is invalid until the template is instantiated

[erichkeane]

lib/Parse/ParseStmt.cpp

186

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.

test/SemaCXX/MicrosoftCompatibility.cpp

222

Ah, right! I'll add those tests as well!

[efriedma]

lib/Parse/ParseStmt.cpp

186

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".

[erichkeane]

lib/Parse/ParseStmt.cpp

186

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!

[erichkeane]

Well hmm... changing this from MSVC to always caused a ton of regressions. I no longer think that this is a proper patch as it sits. Additionally, it doesn't fix the A<T>::TYPE *var3 condition.

[erichkeane]

I don't really see a good solution for this, and haven't tried in a while, so I'll see if I can prep another version of this in the future.

lib/Parse/ParseStmt.cpp

case tok::identifier: {
     Token Next = NextToken();
     if (Next.is(tok::colon)) { // C99 6.8.1: labeled-statement
       // identifier ':' statement
       return ParseLabeledStatement(Attrs);
     }
 
     // Look up the identifier, and typo-correct it to a keyword if it's not
     // found.
-    if (Next.isNot(tok::coloncolon)) {
+    if (Next.isNot(tok::coloncolon) && (!getLangOpts().MSVCCompat ||
+        Next.isNot(tok::less))) {
       // Try to limit which sets of keywords should be included in typo
       // correction based on what the next token is.
       if (TryAnnotateName(/*IsAddressOfOperand*/ false,
                           llvm::make_unique<StatementFilterCCC>(Next)) ==
           ANK_Error) {
         // Handle errors here by skipping up to the next semicolon or '}', and
         // eat the semicolon if that's what stopped us.
         SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);

test/SemaCXX/MicrosoftCompatibility.cpp

 public:
     typedef int Type;
 };
 
 template <class T>
 void function_missing_typename(const T::Type param)// expected-warning {{missing 'typename' prior to dependent type name}}
 {
     const T::Type var = 2; // expected-warning {{missing 'typename' prior to dependent type name}}
+    const A<T>::TYPE var2 = 2; // expected-warning {{missing 'typename' prior to dependent type name}}
+    A<T>::TYPE var3 = 2; // expected-warning {{missing 'typename' prior to dependent type name}}
+    MissingTypename::A<T>::TYPE var4 = 2; // expected-warning {{missing 'typename' prior to dependent type name}}
 }
 
 template void function_missing_typename<D>(const D::Type param);
 
 }
 
 enum ENUM2 {
   ENUM2_a = (enum ENUM2) 4,