Re: [C++ Patch] PR 71248

[Jason Merrill]

OK.

Jason


On Tue, May 31, 2016 at 8:54 AM, Paolo Carlini  wrote:
> Hi,
>
> the primary issue is already fixed, we don't ICE anymore, but the location
> of the error message is (badly) incorrect, because
> check_static_variable_definition doesn't use DECL_SOURCE_LOCATION. Just
> doing that, consistently, plus a number of additional column checks in
> existing testcases amounts to most of the patch. There is a subtlety though:
> as shown by constexpr-static8.C we have been giving redundant diagnostics
> about the out-of-class definitions of the erroneous in-class declarations.
> This is particularly evident now because we would have to get right the
> location of the latter too and DECL_SOURCE_LOCATION doesn't help much for
> those. However, I think we simply want to suppress those messages, thus the
> early return at the beginning of check_static_variable_definition. The
> latter is called only by cp_finish_decl, thus conceivably the same logic
> could be easily shuffled around
>
> Tested x86_64-linux.
>
> Thanks,
> Paolo.
>
> ///

[C++ Patch] PR 71248

[Paolo Carlini]

Hi,

the primary issue is already fixed, we don't ICE anymore, but the 
location of the error message is (badly) incorrect, because 
check_static_variable_definition doesn't use DECL_SOURCE_LOCATION. Just 
doing that, consistently, plus a number of additional column checks in 
existing testcases amounts to most of the patch. There is a subtlety 
though: as shown by constexpr-static8.C we have been giving redundant 
diagnostics about the out-of-class definitions of the erroneous in-class 
declarations. This is particularly evident now because we would have to 
get right the location of the latter too and DECL_SOURCE_LOCATION 
doesn't help much for those. However, I think we simply want to suppress 
those messages, thus the early return at the beginning of 
check_static_variable_definition. The latter is called only by 
cp_finish_decl, thus conceivably the same logic could be easily shuffled 
around


Tested x86_64-linux.

Thanks,
Paolo.

///
/cp
2016-05-31  Paolo Carlini  

PR c++/71248
* decl.c (check_static_variable_definition): Use DECL_SOURCE_LOCATION
to obtain correct locations; avoid redundant diagnostics on
out-of-class definitions.

/testsuite
2016-05-31  Paolo Carlini  

PR c++/71248
* g++.dg/cpp0x/pr71248.C: New.
* g++.dg/cpp0x/auto7.C: Test column numbers too.
* g++.dg/cpp0x/constexpr-static8.C: Likewise.
* g++.dg/init/new37.C: Likewise.
* g++.dg/template/static1.C: Likewise.
* g++.dg/template/static2.C: Likewise.
Index: cp/decl.c
===
--- cp/decl.c   (revision 236910)
+++ cp/decl.c   (working copy)
@@ -8581,6 +8581,9 @@ build_ptrmem_type (tree class_type, tree member_ty
 static int
 check_static_variable_definition (tree decl, tree type)
 {
+  /* Avoid redundant diagnostics on out-of-class definitions.  */
+  if (!current_class_type || !TYPE_BEING_DEFINED (current_class_type))
+return 0;
   /* Can't check yet if we don't know the type.  */
   if (dependent_type_p (type))
 return 0;
@@ -8591,15 +8594,17 @@ check_static_variable_definition (tree decl, tree
   else if (cxx_dialect >= cxx11 && !INTEGRAL_OR_ENUMERATION_TYPE_P (type))
 {
   if (!COMPLETE_TYPE_P (type))
-   error ("in-class initialization of static data member %q#D of "
-  "incomplete type", decl);
+   error_at (DECL_SOURCE_LOCATION (decl),
+ "in-class initialization of static data member %q#D of "
+ "incomplete type", decl);
   else if (literal_type_p (type))
-   permerror (input_location,
+   permerror (DECL_SOURCE_LOCATION (decl),
   "% needed for in-class initialization of "
   "static data member %q#D of non-integral type", decl);
   else
-   error ("in-class initialization of static data member %q#D of "
-  "non-literal type", decl);
+   error_at (DECL_SOURCE_LOCATION (decl),
+ "in-class initialization of static data member %q#D of "
+ "non-literal type", decl);
   return 1;
 }
 
@@ -8611,17 +8616,20 @@ check_static_variable_definition (tree decl, tree
  required.  */
   if (!ARITHMETIC_TYPE_P (type) && TREE_CODE (type) != ENUMERAL_TYPE)
 {
-  error ("invalid in-class initialization of static data member "
-"of non-integral type %qT",
-type);
+  error_at (DECL_SOURCE_LOCATION (decl),
+   "invalid in-class initialization of static data member "
+   "of non-integral type %qT",
+   type);
   return 1;
 }
   else if (!CP_TYPE_CONST_P (type))
-error ("ISO C++ forbids in-class initialization of non-const "
-  "static member %qD",
-  decl);
+error_at (DECL_SOURCE_LOCATION (decl),
+ "ISO C++ forbids in-class initialization of non-const "
+ "static member %qD",
+ decl);
   else if (!INTEGRAL_OR_ENUMERATION_TYPE_P (type))
-pedwarn (input_location, OPT_Wpedantic, "ISO C++ forbids initialization of 
member constant "
+pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic,
+"ISO C++ forbids initialization of member constant "
 "%qD of non-integral type %qT", decl, type);
 
   return 0;
Index: testsuite/g++.dg/cpp0x/auto7.C
===
--- testsuite/g++.dg/cpp0x/auto7.C  (revision 236910)
+++ testsuite/g++.dg/cpp0x/auto7.C  (working copy)
@@ -7,7 +7,7 @@ auto j; // { dg-error "has no initializer" }
 
 template struct A
 {
-  static auto k = 7;   // { dg-error "non-const" }
+  static auto k = 7;   // { dg-error "15:ISO C\\+\\+ forbids" }
   static auto l;   // { dg-error "has no initializer" }
   auto m;  // { dg-error "non-static data member declared" }
 };
Index: