diff mbox

[C++] PR 71248

Message ID 574D89A0.70309@oracle.com
State New
Headers show

Commit Message

Paolo Carlini May 31, 2016, 12:54 p.m. UTC 
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  <paolo.carlini@oracle.com>

	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  <paolo.carlini@oracle.com>

	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.

Comments

Jason Merrill May 31, 2016, 2:20 p.m. UTC | #1 
OK.

Jason


On Tue, May 31, 2016 at 8:54 AM, Paolo Carlini <paolo.carlini@oracle.com> 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.
>
> ///////////////////////
diff mbox

Patch

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),
 		   "%<constexpr%> 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<int> 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: testsuite/g++.dg/cpp0x/constexpr-static8.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-static8.C	(revision 236910)
+++ testsuite/g++.dg/cpp0x/constexpr-static8.C	(working copy)
@@ -3,6 +3,6 @@ 
 // { dg-options "-fpermissive" }
 
 struct Foo {
-  static const double d = 3.14; // { dg-warning "constexpr" }
+  static const double d = 3.14; // { dg-warning "23:'constexpr' needed" }
 };
-const double Foo::d;            // { dg-warning "constexpr" }
+const double Foo::d;
Index: testsuite/g++.dg/cpp0x/pr71248.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71248.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr71248.C	(working copy)
@@ -0,0 +1,10 @@ 
+// PR c++/71248
+// { dg-do compile { target c++11 } }
+
+struct S
+{
+    int a;
+    static int S::*typeMembers[] = {  // { dg-error "20:in-class initialization" }
+        &S::a,
+    };
+};
Index: testsuite/g++.dg/init/new37.C
===================================================================
--- testsuite/g++.dg/init/new37.C	(revision 236910)
+++ testsuite/g++.dg/init/new37.C	(working copy)
@@ -40,7 +40,8 @@  struct T1 {
 };
 
 struct T2 {
-  static const double n = 2; // { dg-error "non-integral type" }
+  static const double n = 2; // { dg-error "23:'constexpr' needed" "" { target c++11 } }
+  // { dg-error "23:ISO C\\+\\+ forbids" "" { target c++98_only } 43 }
 };
 
 struct T3 {
Index: testsuite/g++.dg/template/static1.C
===================================================================
--- testsuite/g++.dg/template/static1.C	(revision 236910)
+++ testsuite/g++.dg/template/static1.C	(working copy)
@@ -1,4 +1,6 @@ 
 template <typename T> struct A
 {
-  static const int t[1][1]={{0}}; // { dg-error "brace-enclosed|in-class" }
+  static const int t[1][1]={{0}}; // { dg-error "20:'constexpr' needed" "" { target c++11 } }
+  // { dg-error "20:invalid in-class" "" { target c++98_only } 3 }
+  // { dg-error "28:a brace-enclosed" "" { target c++98_only } 3 }
 };
Index: testsuite/g++.dg/template/static2.C
===================================================================
--- testsuite/g++.dg/template/static2.C	(revision 236910)
+++ testsuite/g++.dg/template/static2.C	(working copy)
@@ -4,7 +4,9 @@  template<int A::* P>
 class B
 {
 public:
-  static int A::* const p = P; // { dg-error "" }
+  static int A::* const p = P; // { dg-error "25:'constexpr' needed" "" { target c++11 } }
+  // { dg-error "25:invalid in-class" "" { target c++98_only } 7 }
+  // { dg-error "29:template parameter" "" { target c++98_only } 7 }
 };
 
 class A