Allow 'static' storage specifier on an out-of-line member function template declaration in MSVCCompat mode

[Manna]

Summary

Microsoft compiler permits the use of 'static' storage specifier outside of a class definition if it's on an out-of-line member function template declaration.

This patch allows 'static' storage specifier on an out-of-line member function template declaration with a warning in Clang (To be compatible with Microsoft).

Intel C/C++ compiler allows the 'static' keyword with a warning in Microsoft mode. GCC allows this with -fpermissive.

[mibintc]

More info: gcc allows this with -fpermissive, Microsoft gives no message regardless of /permissive-

[aaron.ballman]

include/clang/Basic/DiagnosticSemaKinds.td

1593 ↗

(On Diff #180786)

Should be an ExtWarn<> instead of Warning<> and should be named ext_static_out_of_line.

1594 ↗

(On Diff #180786)

Nit: instead of duplicating the text, you can use err_static_out_of_line.Text instead.

lib/Sema/SemaDecl.cpp

8629 ↗

(On Diff #180786)

of 'static' -> of a 'static'

8633 ↗

(On Diff #180786)

I would use a ternary operator here instead of duplicating the code. e.g.,

Diag(D.getDeclSpec().getStorageClassSpecLoc(),
     NewFD->getDescribedFunctionTemplate() && getLangOpts().MSVCCompat ? diag::warn_static_out_of_line : diag::err_static_out_of_line)
  << FixItHint::CreateRemoval(D.getDeclSpec().getStorageClassSpecLoc());

(only with better formatting)

test/SemaCXX/warn-static-outside-class-definition.cpp

1 ↗

(On Diff #180786)

Remove -fms-extensions from the RUN line as this is a compatibility feature

3 ↗

(On Diff #180786)

You should run this through clang-format to match the formatting guidelines.

[rnk]

Functionality seems reasonable, I'll let @aaron.ballman finish the review.

[Manna]

Thanks for the feedback. Patch is updated based on review comments.

[aaron.ballman]

LGTM!

[lebedev.ri]

I feel like pointing out that this review happened with no lists subscribed, so it is practically invalid.

[rnk]

In D56473#1371539, @lebedev.ri wrote:

I feel like pointing out that this review happened with no lists subscribed, so it is practically invalid.

Fair enough, but there were two appropriate reviewers who approved it. I think it was an honest, fairly common mistake. Unless you have objections, I don't see a need to revert and have more discussion.

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

def err_explicit_non_function : Error<
   "'explicit' can only appear on non-static member functions">;
 def err_explicit_out_of_class : Error<
   "'explicit' can only be specified inside the class definition">;
 def err_explicit_non_ctor_or_conv_function : Error<
   "'explicit' can only be applied to a constructor or conversion function">;
 def err_static_not_bitfield : Error<"static member %0 cannot be a bit-field">;
 def err_static_out_of_line : Error<
   "'static' can only be specified inside the class definition">;
+def ext_static_out_of_line : ExtWarn<
+  err_static_out_of_line.Text>,
+  InGroup<MicrosoftTemplate>;
 def err_storage_class_for_static_member : Error<
   "static data member definition cannot specify a storage class">;
 def err_typedef_not_bitfield : Error<"typedef member %0 cannot be a bit-field">;
 def err_not_integral_type_bitfield : Error<
   "bit-field %0 has non-integral type %1">;
 def err_not_integral_type_anon_bitfield : Error<
   "anonymous bit-field has non-integral type %0">;
 def err_anon_bitfield_qualifiers : Error<

cfe/trunk/lib/Sema/SemaDecl.cpp

if (SC == SC_Static && isa<CXXMethodDecl>(NewFD) &&
         !CurContext->isRecord()) {
       // C++ [class.static]p1:
       //   A data or function member of a class may be declared static
       //   in a class definition, in which case it is a static member of
       //   the class.
 
       // Complain about the 'static' specifier if it's on an out-of-line
       // member function definition.
+
+      // MSVC permits the use of a 'static' storage specifier on an out-of-line
+      // member function template declaration, warn about this.
       Diag(D.getDeclSpec().getStorageClassSpecLoc(),
-           diag::err_static_out_of_line)
+           NewFD->getDescribedFunctionTemplate() && getLangOpts().MSVCCompat
+           ? diag::ext_static_out_of_line : diag::err_static_out_of_line)
         << FixItHint::CreateRemoval(D.getDeclSpec().getStorageClassSpecLoc());
     }
 
     // C++11 [except.spec]p15:
     //   A deallocation function with no exception-specification is treated
     //   as if it were specified with noexcept(true).
     const FunctionProtoType *FPT = R->getAs<FunctionProtoType>();
     if ((Name.getCXXOverloadedOperator() == OO_Delete ||

cfe/trunk/test/SemaCXX/warn-static-outside-class-definition.cpp

+// RUN: %clang_cc1 -fms-compatibility -fsyntax-only -verify %s
+
+struct C {
+  template <typename T> static int foo(T);
+};
+
+template <typename T> static int C::foo(T) { 
+  //expected-warning@-1 {{'static' can only be specified inside the class definition}}
+  return 0;
+}
+