[ms] Allow more unqualified lookup of types in dependent base classes

[rnk]

Summary

In dependent contexts where we know a type name is required, such as a
new expression, we can recover by forming a DependentNameType.

Works towards parsing atlctrlw.h, which is PR26748.

[rsmith]

lib/Parse/ParseDecl.cpp

2299

Maybe put the check here instead. That way, you can apply it for any DSContext.

lib/Sema/SemaDecl.cpp

507

Missing loop termination condition

[rnk]

lib/Parse/ParseDecl.cpp

2299

Done. With that change, we ended up over-accepting invalid C++ like this:

template <typename T>
struct A : T {
  C c; // MSVC rejects, C is undeclared. My patch accepted invalid.
};
struct B { typedef int C; };
template struct A<B>;

This recovery mode should only be firing inside *methods* of classes with dependent bases, so I tweaked ActOnMSVCUnknownType to do that.

[rnk]

• Recover in ParseImplicitInt instead

[rsmith]

include/clang/Sema/Sema.h

1536–1537

Can you remove this now, along with the corresponding recovery path in the parser? The new recovery path seems like it should be able to cover a superset of cases.

lib/Sema/SemaDecl.cpp

524–527

Instead of doing this, could you synthesizeCurrentNestedNameSpecifier as ActOnDelayedDefaultTemplateArg does, and just check for the existence of a dependent base class here? That way you should do the right thing even if there are multiple dependent base classes.

[rnk]

• Share the MSVC compatibility hack between the inside a method case and the default type template argument case

[rsmith]

include/clang/Sema/Sema.h

1539

form -> form a

lib/Sema/SemaDecl.cpp

512

refering -> referring

532–534

Maybe make this an else for the above if.

573–576

Doesn't seem to be done? I can easily believe there's a good reason why we want two different paths for building a recovery nested name specifier, but I'd like to know what it is at least.

[rnk]

lib/Sema/SemaDecl.cpp

573–576

We should already be handling the case you are thinking of. We don't recover by using a dependent base in the NNS. We recover by using the class *deriving* from those bases in the NNS, like this:

template <typename T, typename U>
struct A : T, U {
  void *p() { return new /*A::*/ X; } // We hallucinate the commented out specifier
};

synthesizeCurrentNestedNameSpecifier is doing something different. It's just looking for the nearest parent scope that can legitimately form an NNS, which means global scope, namespace scope, or record scope, mainly by skipping function scopes. This is sort of what it looks like:

template <typename T = /*::*/ X> struct A { ... };
namespace ns {
template <typename T = /*ns::*/ X> struct B { ... };
}

[rnk]

• nits

[rnk]

• fix the diff

include/clang/Sema/Sema.h

void DiagnoseUnknownTypeName(IdentifierInfo *&II,
                                CXXScopeSpec *SS,
                                ParsedType &SuggestedType,
                                bool AllowClassTemplates = false);
 
   /// \brief For compatibility with MSVC, we delay parsing of some default
   /// template type arguments until instantiation time.  Emits a warning and
   /// returns a synthesized DependentNameType that isn't really dependent on any
   /// other template arguments.
   ParsedType ActOnDelayedDefaultTemplateArg(const IdentifierInfo &II,
                                             SourceLocation NameLoc);
+  ParsedType ActOnMSVCUnknownTypeName(const IdentifierInfo &II,
+                                      SourceLocation NameLoc);
 
   /// \brief Describes the result of the name lookup and resolution performed
   /// by \c ClassifyName().
   enum NameClassificationKind {
     NC_Unknown,
     NC_Error,
     NC_Keyword,
     NC_Type,

lib/Parse/ParseDecl.cpp

if (getLangOpts().CPlusPlus &&
       DS.getStorageClassSpec() == DeclSpec::SCS_auto) {
     // Don't require a type specifier if we have the 'auto' storage class
     // specifier in C++98 -- we'll promote it to a type specifier.
     if (SS)
       AnnotateScopeToken(*SS, /*IsNewAnnotation*/false);
     return false;
   }
 
+  if (getLangOpts().CPlusPlus && (!SS || SS->isEmpty()) &&
+      getLangOpts().MSVCCompat) {
+    // Lookup of an unqualified type name has failed in MSVC compatibility mode.
+    // Give Sema a chance to recover if we are in a template with dependent base
+    // classes.
+    if (ParsedType T = Actions.ActOnMSVCUnknownTypeName(
+            *Tok.getIdentifierInfo(), Tok.getLocation())) {
+      const char *PrevSpec;
+      unsigned DiagID;
+      DS.SetTypeSpecType(DeclSpec::TST_typename, Loc, PrevSpec, DiagID, T,
+                         Actions.getASTContext().getPrintingPolicy());
+      DS.SetRangeEnd(Tok.getLocation());
+      ConsumeToken();
+      return false;
+    }
+  }
+
   // Otherwise, if we don't consume this token, we are going to emit an
   // error anyway.  Try to recover from various common problems.  Check
   // to see if this was a reference to a tag name without a tag specified.
   // This is a common problem in C (saying 'foo' instead of 'struct foo').
   //
   // C++ doesn't need this, and isTagName doesn't take SS.
   if (SS == nullptr) {
     const char *TagName = nullptr, *FixitTagName = nullptr;
     tok::TokenKind TagKind = tok::unknown;

lib/Sema/SemaDecl.cpp

for (;; DC = DC->getLookupParent()) {
     else if (isa<TranslationUnitDecl>(DC))
       return NestedNameSpecifier::GlobalSpecifier(Context);
   }
   llvm_unreachable("something isn't in TU scope?");
 }
 
 ParsedType Sema::ActOnDelayedDefaultTemplateArg(const IdentifierInfo &II,
                                                 SourceLocation NameLoc) {
+  assert(getLangOpts().MSVCCompat && "shouldn't be called in non-MSVC mode");
+
   // Accepting an undeclared identifier as a default argument for a template
   // type parameter is a Microsoft extension.
   Diag(NameLoc, diag::ext_ms_delayed_template_argument) << &II;
 
   // Build a fake DependentNameType that will perform lookup into CurContext at
   // instantiation time.  The name specifier isn't dependent, so template
   // instantiation won't transform it.  It will retry the lookup, however.
   NestedNameSpecifier *NNS =
ParsedType Sema::ActOnDelayedDefaultTemplateArg(const IdentifierInfo &II,
   TypeLocBuilder Builder;
   DependentNameTypeLoc DepTL = Builder.push<DependentNameTypeLoc>(T);
   DepTL.setNameLoc(NameLoc);
   DepTL.setElaboratedKeywordLoc(SourceLocation());
   DepTL.setQualifierLoc(QualifierLoc);
   return CreateParsedType(T, Builder.getTypeSourceInfo(Context, T));
 }
 
+/// Find the parent class with dependent bases of the innermost enclosing method
+/// context. Do not look for enclosing CXXRecordDecls directly, or we will end
+/// up allowing unqualified dependent type names at class-level, which MSVC
+/// correctly rejects.
+static const CXXRecordDecl *
+findRecordWithDependentBasesOfEnclosingMethod(const DeclContext *DC) {
+  for (; DC && DC->isDependentContext(); DC = DC->getLookupParent()) {
+    DC = DC->getPrimaryContext();
+    if (const auto *MD = dyn_cast<CXXMethodDecl>(DC))
+      if (MD->getParent()->hasAnyDependentBases())
+        return MD->getParent();
+  }
+  return nullptr;
+}
+
+ParsedType Sema::ActOnMSVCUnknownTypeName(const IdentifierInfo &II,
+                                          SourceLocation NameLoc) {
+  assert(getLangOpts().MSVCCompat && "shouldn't be called in non-MSVC mode");
+
+  const CXXRecordDecl *RD =
+      findRecordWithDependentBasesOfEnclosingMethod(CurContext);
+  if (!RD)
+    return ParsedType();
+
+  // Diagnose that this identifier was undeclared, and retry the lookup during
+  // template instantiation.
+  Diag(NameLoc, diag::ext_undeclared_unqual_id_with_dependent_base) << &II
+                                                                    << RD;
+
+  // Build a DependentNameType that will perform lookup into RD at instantiation
+  // time.
+  NestedNameSpecifier *NNS = NestedNameSpecifier::Create(
+      Context, nullptr, RD->isTemplateDecl(), RD->getTypeForDecl());
+  QualType T = Context.getDependentNameType(ETK_None, NNS, &II);
+
+  // Build type location information.  We synthesized the qualifier, so we have
+  // to build a fake NestedNameSpecifierLoc.
+  NestedNameSpecifierLocBuilder NNSLocBuilder;
+  NNSLocBuilder.MakeTrivial(Context, NNS, SourceRange(NameLoc));
+  NestedNameSpecifierLoc QualifierLoc = NNSLocBuilder.getWithLocInContext(Context);
+
+  TypeLocBuilder Builder;
+  DependentNameTypeLoc DepTL = Builder.push<DependentNameTypeLoc>(T);
+  DepTL.setNameLoc(NameLoc);
+  DepTL.setElaboratedKeywordLoc(SourceLocation());
+  DepTL.setQualifierLoc(QualifierLoc);
+  return CreateParsedType(T, Builder.getTypeSourceInfo(Context, T));
+}
+
 /// isTagName() - This method is called *for error recovery purposes only*
 /// to determine if the specified name is a valid tag name ("struct foo").  If
 /// so, this returns the TST for the tag corresponding to it (TST_enum,
 /// TST_union, TST_struct, TST_interface, TST_class).  This is used to diagnose
 /// cases in C where the user forgot to specify the tag.
 DeclSpec::TST Sema::isTagName(IdentifierInfo &II, Scope *S) {
   // Do a tag name lookup in this scope.
   LookupResult R(*this, &II, SourceLocation(), LookupTagName);
   LookupName(R, S, false);
   R.suppressDiagnostics();
   if (R.getResultKind() == LookupResult::Found)
     if (const TagDecl *TD = R.getAsSingle<TagDecl>()) {
       switch (TD->getTagKind()) {
       case TTK_Struct: return DeclSpec::TST_struct;
       case TTK_Interface: return DeclSpec::TST_interface;
       case TTK_Union:  return DeclSpec::TST_union;
       case TTK_Class:  return DeclSpec::TST_class;
       case TTK_Enum:   return DeclSpec::TST_enum;
       }
     }
 
   return DeclSpec::TST_unspecified;
 }
 
 /// isMicrosoftMissingTypename - In Microsoft mode, within class scope,
 /// if a CXXScopeSpec's type is equal to the type of one of the base classes
 /// then downgrade the missing typename error to a warning.
 /// This is needed for MSVC compatibility; Example:
 /// @code
 /// template<class T> class A {
 /// public:

test/SemaTemplate/ms-delayed-default-template-args.cpp

 struct Foo {
   static_assert(sizeof(T) == 4, "Bar should have gotten int");
   // FIXME: These diagnostics are bad.
 }; // expected-error {{expected ',' or '>' in template-parameter-list}}
 // expected-warning@-1 {{does not declare anything}}
 typedef int Weber;
 }
 
+// MSVC accepts this, but Clang doesn't.
+namespace test_scope_spec {
+template <typename T = ns::Bar>  // expected-error {{use of undeclared identifier 'ns'}}
+struct Foo {
+  static_assert(sizeof(T) == 4, "Bar should have gotten int");
+};
+namespace ns { typedef int Bar; }
+}
+
 #ifdef __clang__
 // These are negative test cases that MSVC doesn't compile either.  Try to use
 // unique undeclared identifiers so typo correction doesn't find types declared
 // above.
 
 namespace test_undeclared_nontype_parm_type {
 template <Zargon N> // expected-error {{unknown type name 'Zargon'}}
 struct Foo { int x[N]; };

test/SemaTemplate/ms-lookup-template-base-classes.cpp

-// RUN: %clang_cc1 -std=c++1y -fms-compatibility -fno-spell-checking -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -std=c++1y -fms-compatibility -fno-spell-checking -fsyntax-only -verify %s
 
 
 template <class T>
 class A {
 public:
    void f(T a) { }// expected-note 2{{must qualify identifier to find this declaration in dependent base class}}
    void g();// expected-note 2{{must qualify identifier to find this declaration in dependent base class}}
 };

 // Don't delay lookup in SFINAE context.
 template <typename T> decltype(g(T())) check(); // expected-note{{candidate template ignored: substitution failure [with T = int]: use of undeclared identifier 'g'}}
 decltype(check<int>()) x; // expected-error{{no matching function for call to 'check'}}
 
 void h();
 template <typename T> decltype(h(T())) check2(); // expected-note{{candidate template ignored: substitution failure [with T = int]: no matching function for call to 'h'}}
 decltype(check2<int>()) y; // expected-error{{no matching function for call to 'check2'}}
 }
+
+// We also allow unqualified lookup into bases in contexts where the we know the
+// undeclared identifier *must* be a type, such as a new expression or catch
+// parameter type.
+template <typename T>
+struct UseUnqualifiedTypeNames : T {
+  void foo() {
+    void *P = new TheType; // expected-warning {{unqualified lookup}} expected-error {{no type}}
+    size_t x = __builtin_offsetof(TheType, f2); // expected-warning {{unqualified lookup}} expected-error {{no type}}
+    try {
+    } catch (TheType) { // expected-warning {{unqualified lookup}} expected-error {{no type}}
+    }
+    enum E : IntegerType { E0 = 42 }; // expected-warning {{unqualified lookup}} expected-error {{no type}}
+    _Atomic(TheType) a; // expected-warning {{unqualified lookup}} expected-error {{no type}}
+  }
+  void out_of_line();
+};
+template <typename T>
+void UseUnqualifiedTypeNames<T>::out_of_line() {
+  void *p = new TheType; // expected-warning {{unqualified lookup}} expected-error {{no type}}
+}
+struct Base {
+  typedef int IntegerType;
+  struct TheType {
+    int f1, f2;
+  };
+};
+template struct UseUnqualifiedTypeNames<Base>;
+struct BadBase { };
+template struct UseUnqualifiedTypeNames<BadBase>; // expected-note-re 2 {{in instantiation {{.*}} requested here}}