diff mbox series

[C++] Adjust one more error message to use rich_location::add_range

Message ID ec6cb249-47a7-f376-aacf-10b54a245959@oracle.com
State New
Headers show
Series [C++] Adjust one more error message to use rich_location::add_range | expand

Commit Message

Paolo Carlini July 2, 2018, 10:58 a.m. UTC 
Hi,

I was double checking my pending patch and going through the errors we 
emit in decl.c and elsewhere about thread_local and __thread and noticed 
another place, in parser.c, where using rich_location::add_range seems 
natural. Note, we could in principle swap location and 
decl_specs->locations[ds_thread] in the error basing on the gnu bool and 
ensure that the caret always points to __thread. All in all, I don't 
think it's worth it...

Thanks, Paolo.

///////////////
/cp
2018-07-02  Paolo Carlini  <paolo.carlini@oracle.com>

	* parser.c (set_and_check_decl_spec_loc): Use rich_location::add_range
	in error message about __thread and thread_local at the same time.

/testsuite
2018-07-02  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/diagnostic/thread-thread_local.C: New.

Comments

Jason Merrill July 2, 2018, 6:18 p.m. UTC | #1 
OK.

On Mon, Jul 2, 2018 at 6:58 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> I was double checking my pending patch and going through the errors we emit
> in decl.c and elsewhere about thread_local and __thread and noticed another
> place, in parser.c, where using rich_location::add_range seems natural.
> Note, we could in principle swap location and
> decl_specs->locations[ds_thread] in the error basing on the gnu bool and
> ensure that the caret always points to __thread. All in all, I don't think
> it's worth it...
>
> Thanks, Paolo.
>
> ///////////////
>
David Malcolm July 2, 2018, 6:20 p.m. UTC | #2 
On Mon, 2018-07-02 at 12:58 +0200, Paolo Carlini wrote:
> Hi,
> 
> I was double checking my pending patch and going through the errors
> we 
> emit in decl.c and elsewhere about thread_local and __thread and
> noticed 
> another place, in parser.c, where using rich_location::add_range
> seems 
> natural. Note, we could in principle swap location and 
> decl_specs->locations[ds_thread] in the error basing on the gnu bool
> and 
> ensure that the caret always points to __thread. All in all, I don't 
> think it's worth it...
> 
> Thanks, Paolo.

The patch looks good to me (with my "diagnostic messages" maintainer
hat on)

Thanks
Dave
diff mbox series

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 262300)
+++ cp/parser.c	(working copy)
@@ -28377,12 +28377,15 @@  set_and_check_decl_spec_loc (cp_decl_specifier_seq
       else if (ds == ds_thread)
 	{
 	  bool gnu = token_is__thread (token);
+	  gcc_rich_location richloc (location);
 	  if (gnu != decl_specs->gnu_thread_keyword_p)
-	    error_at (location,
-		      "both %<__thread%> and %<thread_local%> specified");
+	    {
+	      richloc.add_range (decl_specs->locations[ds_thread], false);
+	      error_at (&richloc,
+			"both %<__thread%> and %<thread_local%> specified");
+	    }
 	  else
 	    {
-	      gcc_rich_location richloc (location);
 	      richloc.add_fixit_remove ();
 	      error_at (&richloc, "duplicate %qD", token->u.value);
 	    }
Index: testsuite/g++.dg/diagnostic/thread-thread_local.C
===================================================================
--- testsuite/g++.dg/diagnostic/thread-thread_local.C	(nonexistent)
+++ testsuite/g++.dg/diagnostic/thread-thread_local.C	(working copy)
@@ -0,0 +1,13 @@ 
+// { dg-options "-fdiagnostics-show-caret" }
+// { dg-do compile { target c++11 } }
+
+thread_local __thread int a;  // { dg-error "14:both .__thread. and .thread_local. specified" }
+/* { dg-begin-multiline-output "" }
+ thread_local __thread int a;
+ ~~~~~~~~~~~~ ^~~~~~~~
+   { dg-end-multiline-output "" } */
+__thread thread_local int b;  // { dg-error "10:both .__thread. and .thread_local. specified" }
+/* { dg-begin-multiline-output "" }
+ __thread thread_local int b;
+ ~~~~~~~~ ^~~~~~~~~~~~
+   { dg-end-multiline-output "" } */