diff mbox series

[C++] Fix a start_decl location

Message ID b8ecf71b-22aa-cfe8-652a-f4cd56c6a8ed@oracle.com
State New
Headers show
Series [C++] Fix a start_decl location | expand

Commit Message

Paolo Carlini Jan. 6, 2019, 9:47 a.m. UTC 
Hi,

this was supposed to be very straightforward but required a little more. 
A first draft of the patch exploited DECL_SOURCE_LOCATION but that 
failed when I tested the case of a function already defined in class: at 
that point the location of the decl is that of the in class definition 
itself not that of the wrong redeclaration. Thus the use of 
declarator->id_loc. Tested x86_64-linux.

A final note, about a detail already noticed many other times: the 
location we store in such cases is that of K not that of f, and that 
seems suboptimal to me: in principle we should point to f and possibly 
have the wavy queue of the caret going back to K, at least that's what 
clang does... no idea id David has this kind of tweak in his todo list.

Thanks, Paolo.

////////////////////////
/cp
2019-01-06  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (start_decl): Improve permerror location.

/testsuite
2019-01-06  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/diagnostic/out-of-class-redeclaration.C: New.

Comments

Jason Merrill Jan. 8, 2019, 2:36 a.m. UTC | #1 
On 1/6/19 4:47 AM, Paolo Carlini wrote:
> Hi,
> 
> this was supposed to be very straightforward but required a little more. 
> A first draft of the patch exploited DECL_SOURCE_LOCATION but that 
> failed when I tested the case of a function already defined in class: at 
> that point the location of the decl is that of the in class definition 
> itself not that of the wrong redeclaration. Thus the use of 
> declarator->id_loc. Tested x86_64-linux.
> 
> A final note, about a detail already noticed many other times: the 
> location we store in such cases is that of K not that of f, and that 
> seems suboptimal to me: in principle we should point to f and possibly 
> have the wavy queue of the caret going back to K, at least that's what 
> clang does... no idea id David has this kind of tweak in his todo list.

David has done various work with handling of qualified-id locations, so 
I imagine he'll be interested in this issue.

The patch is OK.

Jason
diff mbox series

Patch

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 267600)
+++ cp/decl.c	(working copy)
@@ -5202,7 +5202,8 @@  start_decl (const cp_declarator *declarator,
       if (DECL_EXTERNAL (decl) && ! DECL_TEMPLATE_SPECIALIZATION (decl)
 	  /* Aliases are definitions. */
 	  && !alias)
-	permerror (input_location, "declaration of %q#D outside of class is not definition",
+	permerror (declarator->id_loc,
+		   "declaration of %q#D outside of class is not definition",
 		   decl);
     }
 
Index: testsuite/g++.dg/diagnostic/out-of-class-redeclaration.C
===================================================================
--- testsuite/g++.dg/diagnostic/out-of-class-redeclaration.C	(nonexistent)
+++ testsuite/g++.dg/diagnostic/out-of-class-redeclaration.C	(working copy)
@@ -0,0 +1,13 @@ 
+// Adapted from g++.old-deja/g++.law/arm8.C
+
+struct K {
+  void f(int);
+};
+
+void K::f(int);  // { dg-error "6:declaration of .void K::f\\(int\\). outside of class" }
+
+struct L {
+  void g(int) {}
+};
+
+void L::g(int);  // { dg-error "6:declaration of .void L::g\\(int\\). outside of class" }