[MS] Warn when shadowing template parameters under -fms-compatibility

[rnk]

Summary

C++ does not allow shadowing template parameters, but previously we
allowed it under -fms-extensions. Now this behavior is controlled by
-fms-compatibility, and we emit a -Wmicrosoft-template warning when it
happens.

Fixes PR43265

[thakis]

Nice!

Hopefully this is rare enough that putting it under an existing warning flag won't be too inconvenient.

[aganea]

You probably already know this, but the behavior between MSVC & Clang is different as to the value of the constant (testcase for PR43265).
See: https://godbolt.org/z/y4uvgp
Clang gives 0, while MSVC gives 5. This incompatibility is a potential source of failure, one of our tests broke because of that.

Do you think this patch could/should be back-ported into 9.0.1? At least having the warning gives an indication about a potential failure.

[rnk]

@hans, are we still accepting 9.0.1 patches? I thought we'd already made a release candidate.

[hans]

In D67463#1767919, @rnk wrote:

@hans, are we still accepting 9.0.1 patches? I thought we'd already made a release candidate.

+Tom: ^

[tstellar]

In D67463#1767919, @rnk wrote:

@hans, are we still accepting 9.0.1 patches? I thought we'd already made a release candidate.

I'm still accepting patches until rc2 (which should have been Monday, but was delayed), so I can pull it if you think it is important.

[rnk]

In D67463#1769238, @tstellar wrote:

In D67463#1767919, @rnk wrote:

@hans, are we still accepting 9.0.1 patches? I thought we'd already made a release candidate.

I'm still accepting patches until rc2 (which should have been Monday, but was delayed), so I can pull it if you think it is important.

You know, it's a significant change in behavior. This will cause hard errors for users of -fms-extensions who do not use -fms-compatibility. Maybe it's better to release that changes in the 10.0 release.

[tstellar]

In D67463#1769557, @rnk wrote:

In D67463#1769238, @tstellar wrote:

In D67463#1767919, @rnk wrote:

@hans, are we still accepting 9.0.1 patches? I thought we'd already made a release candidate.

I'm still accepting patches until rc2 (which should have been Monday, but was delayed), so I can pull it if you think it is important.

You know, it's a significant change in behavior. This will cause hard errors for users of -fms-extensions who do not use -fms-compatibility. Maybe it's better to release that changes in the 10.0 release.

Ok, we'll leave it out.

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def err_ovl_no_viable_literal_operator : Error<
   "no matching literal operator for call to %0"
   "%select{| with argument of type %2| with arguments of types %2 and %3}1"
   "%select{| or 'const char *'}4"
   "%select{|, and no matching literal operator template}5">;
 
 // C++ Template Declarations
 def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
+def ext_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Text>, InGroup<MicrosoftTemplate>;
 def note_template_param_here : Note<"template parameter is declared here">;
 def warn_template_export_unsupported : Warning<
   "exported templates are unsupported">;
 def err_template_outside_namespace_or_class_scope : Error<
   "templates can only be declared in namespace or class scope">;
 def err_template_inside_local_class : Error<
   "templates cannot be declared inside of a local class">;
 def err_template_linkage : Error<"templates must have C++ linkage">;

cfe/trunk/lib/Sema/SemaTemplate.cpp

 
 /// DiagnoseTemplateParameterShadow - Produce a diagnostic complaining
 /// that the template parameter 'PrevDecl' is being shadowed by a new
 /// declaration at location Loc. Returns true to indicate that this is
 /// an error, and false otherwise.
 void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) {
   assert(PrevDecl->isTemplateParameter() && "Not a template parameter");
 
-  // Microsoft Visual C++ permits template parameters to be shadowed.
-  if (getLangOpts().MicrosoftExt)
-    return;
-
   // C++ [temp.local]p4:
   //   A template-parameter shall not be redeclared within its
   //   scope (including nested scopes).
-  Diag(Loc, diag::err_template_param_shadow)
-    << cast<NamedDecl>(PrevDecl)->getDeclName();
+  //
+  // Make this a warning when MSVC compatibility is requested.
+  unsigned DiagId = getLangOpts().MSVCCompat ? diag::ext_template_param_shadow
+                                             : diag::err_template_param_shadow;
+  Diag(Loc, DiagId) << cast<NamedDecl>(PrevDecl)->getDeclName();
   Diag(PrevDecl->getLocation(), diag::note_template_param_here);
 }
 
 /// AdjustDeclIfTemplate - If the given decl happens to be a template, reset
 /// the parameter D to reference the templated declaration and return a pointer
 /// to the template declaration. Otherwise, do nothing to D and return null.
 TemplateDecl *Sema::AdjustDeclIfTemplate(Decl *&D) {
   if (TemplateDecl *Temp = dyn_cast_or_null<TemplateDecl>(D)) {

cfe/trunk/test/Parser/DelayedTemplateParsing.cpp

 
 }
 
 template <class T> void foo5() {} //expected-note {{previous definition is here}} 
 template <class T> void foo5() {} // expected-error {{redefinition of 'foo5'}}
 
               
 
-namespace Inner_Outer_same_template_param_name {              
-
-template <class T>
-class Outmost {
-public:
-    template <class T>
-    class Inner {
-    public:
-        void f() {
-            T* var;
-        }
-   };
-};
-
-}
-
 
 namespace PR11931 {
 
 template <typename RunType>
 struct BindState;
 
   template<>
 struct BindState<void(void*)> {

cfe/trunk/test/SemaCXX/MicrosoftCompatibility.cpp

 namespace PR25265 {
 struct S {
   int fn() throw(); // expected-note {{previous declaration is here}}
 };
 
 int S::fn() { return 0; } // expected-warning {{is missing exception specification}}
 }
 
+namespace PR43265 {
+template <int N> // expected-note {{template parameter is declared here}}
+struct Foo {
+  static const int N = 42; // expected-warning {{declaration of 'N' shadows template parameter}}
+};
+}
+
+namespace Inner_Outer_same_template_param_name {
+template <typename T> // expected-note {{template parameter is declared here}}
+struct Outmost {
+  template <typename T> // expected-warning {{declaration of 'T' shadows template parameter}}
+  struct Inner {
+    void f() {
+      T *var;
+    }
+  };
+};
+}