In MSVC compatibility mode, handle unqualified templated base class initialization

[frederic-tingaud-sonarsource]

Summary

Before C++20, MSVC was supporting not mentioning the template argument of the base class when initializing a class inheriting a templated base class.
So the following code compiled correctly:

template <class T>
class Base {
};

template <class T>
class Derived : public Base<T> {
public:
  Derived() : Base() {}
};

void test() {
    Derived<int> d;
}

See https://godbolt.org/z/Pxxe7nccx for a conformance view.

This patch adds support for such construct when in MSVC compatibility mode.

[rnk]

clang/lib/Sema/SemaDeclCXX.cpp

4319–4320

We have to make sure our MS compatibility logic fires *before* we try to correct typos. It's currently a big issue that we don't. You can try adjusting your test case by adding a similarly named class like Baze and see if you still get the desired behavior.

See this old issue from 2014:
https://github.com/llvm/llvm-project/issues/20623

[frederic-tingaud-sonarsource]

Added two similar names in the tests, to try to trick the typo correction.

[rnk]

clang/lib/Sema/SemaDeclCXX.cpp

4319–4320

What I'd like to see is just to move the MSVCCompat check up before the typo correction. Basically, once we start typo correction, we *must* emit an error diagnostic. We may attempt to recover by returning a recovery AST node, but the compilation should fail overall.

I think in practice, we only do typo correction if lookup is empty, and your code only fires if name lookup returns a single result, so it's functionally equivalent to reorder these codepaths. I'd prefer that structure, mostly for readability.

[frederic-tingaud-sonarsource]

Sorry about the confusion, it makes sense. Fixed.

[frederic-tingaud-sonarsource]

By the way, in the current state of the code, there is no risk of such conflict between typo correction and UnqualifiedBase, because when an unqualified base exists the lookup result will not be empty.
That's why the test was not failing even without moving the compatibility first.

[rnk]

lgtm, thanks!

In D124666#3490871, @frederic-tingaud-sonarsource wrote:

By the way, in the current state of the code, there is no risk of such conflict between typo correction and UnqualifiedBase, because when an unqualified base exists the lookup result will not be empty.
That's why the test was not failing even without moving the compatibility first.

Yep, that's exactly it, but with the reordering it's somewhat clearer that, if we make this work for MS compat purposes, typo correction will not occur.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_found_in_dependent_base : Error<
   "explicit qualification required to use member %0 from dependent base class">;
 def ext_found_in_dependent_base : ExtWarn<"use of member %0 "
   "found via unqualified lookup into dependent bases of class templates is a "
   "Microsoft extension">, InGroup<MicrosoftTemplate>;
 def err_found_later_in_class : Error<"member %0 used before its declaration">;
 def ext_found_later_in_class : ExtWarn<
   "use of member %0 before its declaration is a Microsoft extension">,
   InGroup<MicrosoftTemplate>;
+def ext_unqualified_base_class : ExtWarn<
+  "unqualified base initializer of class templates is a Microsoft extension">,
+  InGroup<MicrosoftTemplate>;
 def note_dependent_member_use : Note<
   "must qualify identifier to find this declaration in dependent base class">;
 def err_not_found_by_two_phase_lookup : Error<"call to function %0 that is neither "
     "visible in the template definition nor found by argument-dependent lookup">;
 def note_not_found_by_two_phase_lookup : Note<"%0 should be declared prior to the "
     "call site%select{| or in %2| or in an associated namespace of one of its arguments}1">;
 def err_undeclared_use : Error<"use of undeclared %0">;
 def warn_deprecated : Warning<"%0 is deprecated">,

clang/lib/Sema/SemaDeclCXX.cpp

if (!TyD) {
             TL.setQualifierLoc(SS.getWithLocInContext(Context));
           }
 
           R.clear();
           R.setLookupName(MemberOrBase);
         }
       }
 
+      if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
+        auto UnqualifiedBase = R.getAsSingle<ClassTemplateDecl>();
+        if (UnqualifiedBase) {
+          Diag(IdLoc, diag::ext_unqualified_base_class)
+              << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+          BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+        }
+      }
+
       // If no results were found, try to correct typos.
       TypoCorrection Corr;
       MemInitializerValidatorCCC CCC(ClassDecl);
       if (R.empty() && BaseType.isNull() &&
           (Corr = CorrectTypo(R.getLookupNameInfo(), R.getLookupKind(), S, &SS,
                               CCC, CTK_ErrorRecovery, ClassDecl))) {
         if (FieldDecl *Member = Corr.getCorrectionDeclAs<FieldDecl>()) {
           // We have found a non-static data member with a similar
           // name to what was typed; complain and initialize that
           // member.

clang/test/SemaTemplate/ms-unqualified-base-class.cpp

+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fsyntax-only -verify=before,expected %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fdelayed-template-parsing -fsyntax-only -verify=before,expected %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fsyntax-only -verify=after,expected %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fdelayed-template-parsing -fsyntax-only -verify=after,expected %s
+
+template <class T>
+class Base {
+};
+
+template <class T>
+class Based {}; // Trying to trick the typo detection
+
+template <class T>
+class Derived : public Base<T> {
+public:
+  // after-error@+1 {{member initializer 'Base' does not name a non-static data member or base class}}
+  Derived() : Base() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+private:
+  int Baze; // Trying to trick the typo detection
+};
+
+template <class T> struct AggregateBase {
+  T i;
+};
+
+template <class T>
+struct AggregateDerived : public AggregateBase<T> {
+  int i;
+
+  // after-error@+1 {{member initializer 'AggregateBase' does not name a non-static data member or base class}}
+  AggregateDerived(T j) : AggregateBase{4}, i{j} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+  int f() {
+    return i + AggregateBase::i; // expected-warning {{use of undeclared identifier 'AggregateBase'; unqualified lookup into dependent bases of class template 'AggregateDerived' is a Microsoft extension}}
+  }
+};
+
+template <class T, typename U> struct MultiTypesBase {
+};
+
+template <class T, class U>
+struct MultiTypesDerived : public MultiTypesBase<T, U> {
+  // after-error@+1 {{member initializer 'MultiTypesBase' does not name a non-static data member or base class}}
+  MultiTypesDerived() : MultiTypesBase{} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template <int I> struct IntegerBase {
+};
+
+template <int I>
+struct IntegerDerived : public IntegerBase<I> {
+  // after-error@+1 {{member initializer 'IntegerBase' does not name a non-static data member or base class}}
+  IntegerDerived() : IntegerBase{} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template <class T> struct ConformingBase {
+  T i;
+};
+
+template <class T>
+struct ConformingDerived : public ConformingBase<T> {
+  int i;
+
+  ConformingDerived(T j) : ConformingBase<T>{4}, i{j} {}
+  int f() {
+    return i + ConformingBase<T>::i;
+  }
+};
+
+int main() {
+  int I;
+  Derived<int> t;
+
+  AggregateDerived<int> AD{2};
+  AD.AggregateBase::i = 3;
+  I = AD.f();
+
+  MultiTypesDerived<int, double> MTD;
+
+  IntegerDerived<4> ID;
+
+  ConformingDerived<int> CD{2};
+  I = CD.f();
+
+  return I;
+}