[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource created this revision.
frederic-tingaud-sonarsource added a reviewer: rnk.
Herald added a project: All.
frederic-tingaud-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I introduced a patch to handle unqualified templated base class initialization 
in MSVC compatibility mode: 
https://reviews.llvm.org/rGc894e85fc64dd8d83b460de81080fff93c5ca334
We identified a problem with this patch in the case where the base class is 
partially specialized, which can lead to triggering an assertion in the case of 
a mix between types and values.
The minimal test case is:

  cpp
  template  class Vec {};
  
  template  class Index : public Vec {
Index() : Vec() {}
  };
  template class Index<0>;

The detailed problem is that I was using the InjectedClassNameSpecialization, 
to which the class template arguments were then applied in order. But in the 
process, we were losing all the partial specializations of the base class and 
creating an index mismatch between the expected and passed arguments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130709

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaTemplate/ms-unqualified-base-class.cpp


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template 
is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 
'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for 
class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not 
name a non-static data member or base class}}
+};
+
+template class Wrong3;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4308,11 +4308,21 @@
   }
 
   if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
-auto UnqualifiedBase = R.getAsSingle();
-if (UnqualifiedBase) {
-  Diag(IdLoc, diag::ext_unqualified_base_class)
-  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
-  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+if (auto UnqualifiedBase = R.getAsSingle()) {
+  auto *TempSpec = cast(
+  UnqualifiedBase->getInjectedClassNameSpecialization());
+  TemplateName TN = TempSpec->getTemplateName();
+  for (auto const &Base : ClassDecl->bases()) {
+QualType BT = cast(Base.getType())->getNamedType();
+const auto *BaseTemplate = 
dyn_cast(BT);
+if (BaseTemplate && Context.hasSameTemplateName(
+BaseTemplate->getTemplateName(), TN)) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = BT;
+  break;
+}
+  }
 }
   }
 


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class 

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4316
+  for (auto const &Base : ClassDecl->bases()) {
+QualType BT = cast(Base.getType())->getNamedType();
+const auto *BaseTemplate = 
dyn_cast(BT);

Is this cast always safe? Is there a simpler way to check if the base type is a 
template specialization type?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130709/new/

https://reviews.llvm.org/D130709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource updated this revision to Diff 448549.
frederic-tingaud-sonarsource added a comment.

Use getAs to see through ElaboratedType as recommended by 
https://reviews.llvm.org/D112374


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130709/new/

https://reviews.llvm.org/D130709

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaTemplate/ms-unqualified-base-class.cpp


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template 
is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 
'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for 
class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not 
name a non-static data member or base class}}
+};
+
+template class Wrong3;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4308,11 +4308,21 @@
   }
 
   if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
-auto UnqualifiedBase = R.getAsSingle();
-if (UnqualifiedBase) {
-  Diag(IdLoc, diag::ext_unqualified_base_class)
-  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
-  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+if (auto UnqualifiedBase = R.getAsSingle()) {
+  auto *TempSpec = cast(
+  UnqualifiedBase->getInjectedClassNameSpecialization());
+  TemplateName TN = TempSpec->getTemplateName();
+  for (auto const &Base : ClassDecl->bases()) {
+auto BaseTemplate =
+Base.getType()->getAs();
+if (BaseTemplate && Context.hasSameTemplateName(
+BaseTemplate->getTemplateName(), TN)) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = Base.getType();
+  break;
+}
+  }
 }
   }
 


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not name a non-static data member or base class}}
+};
+
+template class Wrong3;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4308,11 +4308,21 @@
   }
 
   if (getLangOpts().MSVCCompat && !getLangOpt

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource added a comment.

@rnk , does this change answer your points?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130709/new/

https://reviews.llvm.org/D130709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Yes, looks good to me, thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130709/new/

https://reviews.llvm.org/D130709

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Balázs Benics via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba1c396e09a6: MSVC compatibility mode: fix error on 
unqualified templated base class… (authored by frederic-tingaud-sonarsource, 
committed by steakhal).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130709/new/

https://reviews.llvm.org/D130709

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaTemplate/ms-unqualified-base-class.cpp


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template 
is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 
'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for 
class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not 
name a non-static data member or base class}}
+};
+
+template class Wrong3;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4309,11 +4309,21 @@
   }
 
   if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
-auto UnqualifiedBase = R.getAsSingle();
-if (UnqualifiedBase) {
-  Diag(IdLoc, diag::ext_unqualified_base_class)
-  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
-  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+if (auto UnqualifiedBase = R.getAsSingle()) {
+  auto *TempSpec = cast(
+  UnqualifiedBase->getInjectedClassNameSpecialization());
+  TemplateName TN = TempSpec->getTemplateName();
+  for (auto const &Base : ClassDecl->bases()) {
+auto BaseTemplate =
+Base.getType()->getAs();
+if (BaseTemplate && Context.hasSameTemplateName(
+BaseTemplate->getTemplateName(), TN)) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = Base.getType();
+  break;
+}
+  }
 }
   }
 


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not name a non-static data member or base class}}
+};
+
+template class Wrong3;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4