[RFC C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386)

[RFC C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386)

[Jakub Jelinek]

Hi!

This PR is complaining about range covering the first token from
an id-expression:
pr87386.C:4:15: error: static assertion failed: foo
4 | static_assert(foo::test<int>::value, "foo");
  |               ^~~
The following patch adjust that to:
pr87386.C:4:31: error: static assertion failed: foo
    4 | static_assert(foo::test<int>::value, "foo");
      |               ~~~~~~~~~~~~~~~~^~~~~
instead, though as the changes to the testsuite show, not really sure
if it is a good idea in all cases, because then we sometimes print:
... bar is not in foo namespace, did you mean 'baz'
  foo::bar
  ~~~~~^~~
  baz
where the baz is misaligned.  Would it be better to just print
pr87386.C:4:31: error: static assertion failed: foo
    4 | static_assert(foo::test<int>::value, "foo");
      |                               ^~~~~
instead?  That would mean dropping the cp_parser_id_expression change
and readjusting or dropping some testsuite changes.

2018-11-20  Jakub Jelinek  <jakub@redhat.com>

	PR c++/87386
	* parser.c (cp_parser_primary_expression): Use
	id_expression.get_location () instead of id_expr_token->location.
	(cp_parser_id_expression): For nested_name_specifier_p, make a
	range covering whole id-expression.
	(cp_parser_operator): For operator "" make a range from "" to
	the end of the suffix with caret at the start of the id.
gcc/testsuite/
	* g++.dg/spellcheck-pr79298.C: Adjust expected diagnostics.
	* g++.dg/lookup/suggestions2.C: Likewise.
	* g++.dg/spellcheck-single-vs-multiple.C: Likewise.
	* g++.dg/parse/error17.C: Likewise.
	* g++.dg/spellcheck-pr77829.C: Likewise.
	* g++.dg/spellcheck-pr78656.C: Likewise.
	* g++.dg/cpp0x/pr51420.C: Likewise.
	* g++.dg/cpp0x/udlit-declare-neg.C: Likewise.
	* g++.dg/cpp0x/udlit-member-neg.C: Likewise.
libstdc++-v3/
	* testsuite/20_util/scoped_allocator/69293_neg.cc: Adjust expected
	line.
	* testsuite/20_util/uses_allocator/cons_neg.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements2.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements3.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements4.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements5.cc: Likewise.

--- gcc/cp/parser.c.jj	2018-11-20 08:41:28.686923718 +0100
+++ gcc/cp/parser.c	2018-11-20 19:05:22.848941522 +0100
@@ -5602,7 +5602,7 @@ cp_parser_primary_expression (cp_parser
 					  /*is_namespace=*/false,
 					  /*check_dependency=*/true,
 					  &ambiguous_decls,
-					  id_expr_token->location);
+					  id_expression.get_location ());
 	    /* If the lookup was ambiguous, an error will already have
 	       been issued.  */
 	    if (ambiguous_decls)
@@ -5673,7 +5673,7 @@ cp_parser_primary_expression (cp_parser
 	    if (parser->local_variables_forbidden_p
 		&& local_variable_p (decl))
 	      {
-		error_at (id_expr_token->location,
+		error_at (id_expression.get_location (),
 			  "local variable %qD may not appear in this context",
 			  decl.get_value ());
 		return error_mark_node;
@@ -5692,7 +5692,7 @@ cp_parser_primary_expression (cp_parser
 		 id_expression.get_location ()));
 	if (error_msg)
 	  cp_parser_error (parser, error_msg);
-	decl.set_location (id_expr_token->location);
+	decl.set_location (id_expression.get_location ());
 	return decl;
       }
 
@@ -5758,6 +5758,7 @@ cp_parser_id_expression (cp_parser *pars
 {
   bool global_scope_p;
   bool nested_name_specifier_p;
+  location_t start_loc = cp_lexer_peek_token (parser->lexer)->location;
 
   /* Assume the `template' keyword was not used.  */
   if (template_p)
@@ -5809,6 +5810,7 @@ cp_parser_id_expression (cp_parser *pars
       parser->object_scope = saved_object_scope;
       parser->qualifying_scope = saved_qualifying_scope;
 
+      unqualified_id.set_range (start_loc, unqualified_id.get_finish ());
       return unqualified_id;
     }
   /* Otherwise, if we are in global scope, then we are looking at one
@@ -14931,7 +14933,7 @@ cp_literal_operator_id (const char* name
 static cp_expr
 cp_parser_operator (cp_parser* parser)
 {
-  tree id = NULL_TREE;
+  cp_expr id = NULL_TREE;
   cp_token *token;
   bool utf8 = false;
 
@@ -15219,8 +15221,9 @@ cp_parser_operator (cp_parser* parser)
 	if (id != error_mark_node)
 	  {
 	    const char *name = IDENTIFIER_POINTER (id);
-	    id = cp_literal_operator_id (name);
+	    *id = cp_literal_operator_id (name);
 	  }
+	id.set_range (start_loc, id.get_finish ());
 	return id;
       }
 
@@ -15244,7 +15247,8 @@ cp_parser_operator (cp_parser* parser)
       id = error_mark_node;
     }
 
-  return cp_expr (id, start_loc);
+  id.set_location (start_loc);
+  return id;
 }
 
 /* Parse a template-declaration.
--- gcc/testsuite/g++.dg/spellcheck-pr79298.C.jj	2018-10-31 10:31:13.281572644 +0100
+++ gcc/testsuite/g++.dg/spellcheck-pr79298.C	2018-11-20 19:14:19.208219955 +0100
@@ -11,7 +11,7 @@ int foo ()
   return M::y; // { dg-error ".y. is not a member of .M." }
   /* { dg-begin-multiline-output "" }
    return M::y;
-             ^
+          ~~~^
      { dg-end-multiline-output "" } */
 }
 
@@ -20,7 +20,7 @@ int bar ()
   return O::colour; // { dg-error ".colour. is not a member of .O.; did you mean 'color'\\?" }
   /* { dg-begin-multiline-output "" }
    return O::colour;
-             ^~~~~~
-             color
+          ~~~^~~~~~
+          color
      { dg-end-multiline-output "" } */
 }
--- gcc/testsuite/g++.dg/lookup/suggestions2.C.jj	2018-10-31 10:31:06.928677642 +0100
+++ gcc/testsuite/g++.dg/lookup/suggestions2.C	2018-11-20 19:12:05.281395810 +0100
@@ -33,8 +33,8 @@ int test_1_long (void) {
   return outer_ns::var_in_inner_ns_a; // { dg-error "did you mean 'var_in_outer_ns'" }
   /* { dg-begin-multiline-output "" }
    return outer_ns::var_in_inner_ns_a;
-                    ^~~~~~~~~~~~~~~~~
-                    var_in_outer_ns
+          ~~~~~~~~~~^~~~~~~~~~~~~~~~~
+          var_in_outer_ns
      { dg-end-multiline-output "" } */
 }
 
@@ -45,7 +45,7 @@ int test_1_short (void) {
   return outer_ns::a; // { dg-error "did you mean 'outer_ns::inner_ns_a::a'" }
   /* { dg-begin-multiline-output "" }
    return outer_ns::a;
-                    ^
+          ~~~~~~~~~~^
      { dg-end-multiline-output "" } */
   // { dg-message "declared here" "" { target *-*-*} decl_of_a }
   /* { dg-begin-multiline-output "" }
@@ -61,8 +61,8 @@ int test_2_long (void) {
   return outer_ns::inner_ns_a::var_in_outer_ns; // { dg-error "did you mean 'var_in_inner_ns_a'" }
   /* { dg-begin-multiline-output "" }
    return outer_ns::inner_ns_a::var_in_outer_ns;
-                                ^~~~~~~~~~~~~~~
-                                var_in_inner_ns_a
+          ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
+          var_in_inner_ns_a
      { dg-end-multiline-output "" } */
 }
 
@@ -73,7 +73,7 @@ int test_2_short (void) {
   return outer_ns::inner_ns_a::o; // { dg-error "did you mean 'outer_ns::o'" }
   /* { dg-begin-multiline-output "" }
    return outer_ns::inner_ns_a::o;
-                                ^
+          ~~~~~~~~~~~~~~~~~~~~~~^
      { dg-end-multiline-output "" } */
   // { dg-message "declared here" "" { target *-*-*} decl_of_o }
   /* { dg-begin-multiline-output "" }
@@ -89,8 +89,8 @@ int test_3_long (void) {
   return outer_ns::inner_ns_a::var_in_inner_ns_b; // { dg-error "did you mean 'var_in_inner_ns_a'" }
   /* { dg-begin-multiline-output "" }
    return outer_ns::inner_ns_a::var_in_inner_ns_b;
-                                ^~~~~~~~~~~~~~~~~
-                                var_in_inner_ns_a
+          ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
+          var_in_inner_ns_a
      { dg-end-multiline-output "" } */
 }
 
@@ -101,7 +101,7 @@ int test_3_short (void) {
   return outer_ns::inner_ns_a::b; // { dg-error "did you mean 'outer_ns::inner_ns_b::b'" }
   /* { dg-begin-multiline-output "" }
    return outer_ns::inner_ns_a::b;
-                                ^
+          ~~~~~~~~~~~~~~~~~~~~~~^
      { dg-end-multiline-output "" } */
   // { dg-message "declared here" "" { target *-*-*} decl_of_b }
   /* { dg-begin-multiline-output "" }
--- gcc/testsuite/g++.dg/spellcheck-single-vs-multiple.C.jj	2018-10-31 10:31:07.765663807 +0100
+++ gcc/testsuite/g++.dg/spellcheck-single-vs-multiple.C	2018-11-20 19:14:35.698952024 +0100
@@ -73,7 +73,7 @@ void test_3 ()
   ns3::goo_3 (); // { dg-error "'goo_3' is not a member of 'ns3'; did you mean 'foo_3'\\?" }
   /* { dg-begin-multiline-output "" }
    ns3::goo_3 ();
-        ^~~~~
-        foo_3
+   ~~~~~^~~~~
+   foo_3
      { dg-end-multiline-output "" } */
 }
--- gcc/testsuite/g++.dg/parse/error17.C.jj	2009-11-27 12:25:22.000000000 +0100
+++ gcc/testsuite/g++.dg/parse/error17.C	2018-11-20 19:12:35.212909527 +0100
@@ -6,4 +6,4 @@ template <typename T> struct B {
 }; 
 struct D : B<int>, B<char> {}; 
  
-int i2 = D::Bar(2); // { dg-error "10:reference to 'Bar' is ambiguous" }
+int i2 = D::Bar(2); // { dg-error "13:reference to 'Bar' is ambiguous" }
--- gcc/testsuite/g++.dg/spellcheck-pr77829.C.jj	2018-10-31 10:31:10.213623350 +0100
+++ gcc/testsuite/g++.dg/spellcheck-pr77829.C	2018-11-20 19:13:30.700008045 +0100
@@ -21,8 +21,8 @@ void fn_1_explicit ()
   detail::some_type i; // { dg-error ".some_type. is not a member of .detail.; did you mean 'some_typedef'\\?" }
   /* { dg-begin-multiline-output "" }
    detail::some_type i;
-           ^~~~~~~~~
-           some_typedef
+   ~~~~~~~~^~~~~~~~~
+   some_typedef
      { dg-end-multiline-output "" } */
 }
 
@@ -47,8 +47,8 @@ void fn_2_explicit (int i) {
   detail::foo(i); // { dg-error ".foo. is not a member of .detail.; did you mean '_foo'\\?" }
   /* { dg-begin-multiline-output "" }
    detail::foo(i);
-           ^~~
-           _foo
+   ~~~~~~~~^~~
+   _foo
      { dg-end-multiline-output "" } */
 }
 
@@ -72,8 +72,8 @@ void fn_3_explicit (int i) {
   detail::something_els(i); // { dg-error ".something_els. is not a member of .detail.; did you mean 'something_else'\\?" }
   /* { dg-begin-multiline-output "" }
    detail::something_els(i);
-           ^~~~~~~~~~~~~
-           something_else
+   ~~~~~~~~^~~~~~~~~~~~~
+   something_else
      { dg-end-multiline-output "" } */
 }
 
@@ -97,7 +97,7 @@ void fn_4_explicit (int i) {
   detail::not_recognized(i); // { dg-error ".not_recognized. is not a member of .detail." }
   /* { dg-begin-multiline-output "" }
    detail::not_recognized(i);
-           ^~~~~~~~~~~~~~
+   ~~~~~~~~^~~~~~~~~~~~~~
      { dg-end-multiline-output "" } */
 }
 
--- gcc/testsuite/g++.dg/spellcheck-pr78656.C.jj	2018-10-31 10:31:10.213623350 +0100
+++ gcc/testsuite/g++.dg/spellcheck-pr78656.C	2018-11-20 19:14:04.161464402 +0100
@@ -7,8 +7,8 @@ void* allocate(std::size_t n)
   return std::allocate<char>().allocate(n); // { dg-error ".allocate. is not a member of .std.; did you mean 'allocator'\\?" }
   /* { dg-begin-multiline-output "" }
    return std::allocate<char>().allocate(n);
-               ^~~~~~~~
-               allocator
+          ~~~~~^~~~~~~~
+          allocator
      { dg-end-multiline-output "" } */
 
   // Various errors follow that we don't care about; suppress them:
@@ -20,8 +20,8 @@ void* test_2(std::size_t n)
   return std::alocator<char>().allocate(n); // { dg-error ".alocator. is not a member of .std.; did you mean 'allocator'\\?" }
   /* { dg-begin-multiline-output "" }
    return std::alocator<char>().allocate(n);
-               ^~~~~~~~
-               allocator
+          ~~~~~^~~~~~~~
+          allocator
      { dg-end-multiline-output "" } */
 
   // Various errors follow that we don't care about; suppress them:
--- gcc/testsuite/g++.dg/cpp0x/pr51420.C.jj	2016-05-30 23:22:54.772099149 +0200
+++ gcc/testsuite/g++.dg/cpp0x/pr51420.C	2018-11-20 19:06:05.236252077 +0100
@@ -3,6 +3,6 @@
 void
 foo()
 {
-  float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
+  float x = operator"" _F();  //  { dg-error  "24:'operator\"\"_F' was not declared in this scope" }
   float y = 0_F;  //  { dg-error  "unable to find numeric literal operator" }
 }
--- gcc/testsuite/g++.dg/cpp0x/udlit-declare-neg.C.jj	2016-05-30 23:22:54.772099149 +0200
+++ gcc/testsuite/g++.dg/cpp0x/udlit-declare-neg.C	2018-11-20 19:07:08.835217615 +0100
@@ -2,14 +2,14 @@
 
 //  Check that undeclared literal operator calls and literals give appropriate errors.
 
-int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
+int i = operator"" _Bar('x');  // { dg-error "20:'operator\"\"_Bar' was not declared in this scope" }
 int j = 'x'_Bar;  // { dg-error "unable to find character literal operator|with|argument" }
 
-int ii = operator"" _BarCharStr("Howdy, Pardner!");  // { dg-error "10:'operator\"\"_BarCharStr' was not declared in this scope" }
+int ii = operator"" _BarCharStr("Howdy, Pardner!");  // { dg-error "21:'operator\"\"_BarCharStr' was not declared in this scope" }
 int jj = "Howdy, Pardner!"_BarCharStr;  // { dg-error "unable to find string literal operator|Possible missing length argument" }
 
-unsigned long long iULL = operator"" _BarULL(666ULL);  // { dg-error "27:'operator\"\"_BarULL' was not declared in this scope" }
+unsigned long long iULL = operator"" _BarULL(666ULL);  // { dg-error "38:'operator\"\"_BarULL' was not declared in this scope" }
 unsigned long long jULL = 666_BarULL;  // { dg-error "unable to find numeric literal operator" }
 
-long double iLD = operator"" _BarLD(666.0L);  // { dg-error "19:'operator\"\"_BarLD' was not declared in this scope" }
+long double iLD = operator"" _BarLD(666.0L);  // { dg-error "30:'operator\"\"_BarLD' was not declared in this scope" }
 long double jLD = 666.0_BarLD;  // { dg-error "unable to find numeric literal operator" }
--- gcc/testsuite/g++.dg/cpp0x/udlit-member-neg.C.jj	2018-06-14 21:01:40.000000000 +0200
+++ gcc/testsuite/g++.dg/cpp0x/udlit-member-neg.C	2018-11-20 19:22:56.470816139 +0100
@@ -7,7 +7,7 @@ public:
   int operator"" _Bar(char32_t);  // { dg-error "7:.int Foo::operator\"\"_Bar\\(char32_t\\). must be a non-member function" }
 };
 
-int i = operator"" _Bar(U'x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
+int i = operator"" _Bar(U'x');  // { dg-error "20:'operator\"\"_Bar' was not declared in this scope" }
 int j = U'x'_Bar;  // { dg-error "unable to find character literal operator" }
 
 int
--- libstdc++-v3/testsuite/20_util/scoped_allocator/69293_neg.cc.jj	2018-07-27 22:20:29.000000000 +0200
+++ libstdc++-v3/testsuite/20_util/scoped_allocator/69293_neg.cc	2018-11-20 21:20:25.123636254 +0100
@@ -46,5 +46,5 @@ test01()
   scoped_alloc sa;
   auto p = sa.allocate(1);
   sa.construct(p);  // this is required to be ill-formed
-  // { dg-error "static assertion failed" "" { target *-*-* } 94 }
+  // { dg-error "static assertion failed" "" { target *-*-* } 96 }
 }
--- libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc.jj	2018-07-27 22:20:29.000000000 +0200
+++ libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc	2018-11-20 21:21:12.868864599 +0100
@@ -43,4 +43,4 @@ void test01()
 
   tuple<Type> t(allocator_arg, a, 1);
 }
-// { dg-error "static assertion failed" "" { target *-*-* } 94 }
+// { dg-error "static assertion failed" "" { target *-*-* } 96 }
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements3.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements3.cc	2018-11-20 21:22:06.049005105 +0100
@@ -21,6 +21,6 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 
 propagate_const<void (*)()> test1;
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements4.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements4.cc	2018-11-20 21:22:29.698622883 +0100
@@ -21,7 +21,7 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 // { dg-error "invalid type" "" { target *-*-* } 66 }
 // { dg-error "uninitialized reference member" "" { target *-*-* } 112 }
 
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements5.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements5.cc	2018-11-20 21:22:45.311370550 +0100
@@ -21,6 +21,6 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 
 propagate_const<int[1]> test1;
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements2.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements2.cc	2018-11-20 21:21:43.959362116 +0100
@@ -21,7 +21,7 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 // { dg-error "not a pointer-to-object type" "" { target *-*-* } 66 }
 // { dg-error "forming pointer to reference type" "" { target *-*-* } 187 }
 // { dg-error "forming pointer to reference type" "" { target *-*-* } 213 }

	Jakub

Re: [RFC C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386)

[David Malcolm]

On Tue, 2018-11-20 at 21:57 +0100, Jakub Jelinek wrote:
> Hi!
> 
> This PR is complaining about range covering the first token from
> an id-expression:
> pr87386.C:4:15: error: static assertion failed: foo
> 4 | static_assert(foo::test<int>::value, "foo");
>   |               ^~~
> The following patch adjust that to:
> pr87386.C:4:31: error: static assertion failed: foo
>     4 | static_assert(foo::test<int>::value, "foo");
>       |               ~~~~~~~~~~~~~~~~^~~~~
> instead, though as the changes to the testsuite show, not really sure
> if it is a good idea in all cases, because then we sometimes print:
> ... bar is not in foo namespace, did you mean 'baz'
>   foo::bar
>   ~~~~~^~~
>   baz
> where the baz is misaligned.  

Is the compiler suggesting the use of
(a) "foo::baz" or 
(b) "::baz"?

Given the underlining, the fix-it hint would be suggesting the
replacement of "foo::bar" with "baz", which would be wrong if we mean
(a) above.

(c.f. "Fix-it hints should work" in https://gcc.gnu.org/onlinedocs/gcci
nt/Guidelines-for-Diagnostics.html )


FWIW in r265610 I ran into issues like this, which I resolved by
holding off on some fix-it hints for the case where we don't have a
location covering the whole of a qualified name.

> Would it be better to just print
> pr87386.C:4:31: error: static assertion failed: foo
>     4 | static_assert(foo::test<int>::value, "foo");
>       |                               ^~~~~
> instead?  That would mean dropping the cp_parser_id_expression change
> and readjusting or dropping some testsuite changes.

That might be better... let me look at the affected test cases.

[...snip...]

 
>  /* Parse a template-declaration.
> --- gcc/testsuite/g++.dg/spellcheck-pr79298.C.jj	2018-10-31
> 10:31:13.281572644 +0100
> +++ gcc/testsuite/g++.dg/spellcheck-pr79298.C	2018-11-20
> 19:14:19.208219955 +0100
> @@ -11,7 +11,7 @@ int foo ()
>    return M::y; // { dg-error ".y. is not a member of .M." }
>    /* { dg-begin-multiline-output "" }
>     return M::y;
> -             ^
> +          ~~~^
>       { dg-end-multiline-output "" } */
>  }

 
> @@ -20,7 +20,7 @@ int bar ()
>    return O::colour; // { dg-error ".colour. is not a member of .O.;
> did you mean 'color'\\?" }
>    /* { dg-begin-multiline-output "" }
>     return O::colour;
> -             ^~~~~~
> -             color
> +          ~~~^~~~~~
> +          color
>       { dg-end-multiline-output "" } */
>  }

This makes the fix-it hint wrong: after the fix-it is applied, it will
become
  return color;
(which won't compile), rather than
  return O::color;
which will.

(I wish we had a good automated way of verifying that fix-it hints fix
things)

> --- gcc/testsuite/g++.dg/lookup/suggestions2.C.jj	2018-10-31
> 10:31:06.928677642 +0100
> +++ gcc/testsuite/g++.dg/lookup/suggestions2.C	2018-11-20
> 19:12:05.281395810 +0100
> @@ -33,8 +33,8 @@ int test_1_long (void) {
>    return outer_ns::var_in_inner_ns_a; // { dg-error "did you mean
> 'var_in_outer_ns'" }
>    /* { dg-begin-multiline-output "" }
>     return outer_ns::var_in_inner_ns_a;
> -                    ^~~~~~~~~~~~~~~~~
> -                    var_in_outer_ns
> +          ~~~~~~~~~~^~~~~~~~~~~~~~~~~
> +          var_in_outer_ns
>       { dg-end-multiline-output "" } */
>  }

Again, this makes the fix-it hint wrong: after the fix-it is applied,
it will become
  return var_in_outer_ns;
(which won't compile) rather than:
  return outer_ns::var_in_outer_ns;

[...snip; I think there are more examples in this test file...] 

> --- gcc/testsuite/g++.dg/spellcheck-single-vs-multiple.C.jj	20
> 18-10-31 10:31:07.765663807 +0100
> +++ gcc/testsuite/g++.dg/spellcheck-single-vs-multiple.C	2018-
> 11-20 19:14:35.698952024 +0100
> @@ -73,7 +73,7 @@ void test_3 ()
>    ns3::goo_3 (); // { dg-error "'goo_3' is not a member of 'ns3';
> did you mean 'foo_3'\\?" }
>    /* { dg-begin-multiline-output "" }
>     ns3::goo_3 ();
> -        ^~~~~
> -        foo_3
> +   ~~~~~^~~~~
> +   foo_3
>       { dg-end-multiline-output "" } */
>  }

Again, the fix-it hint becomes wrong, it will become:
   foo_3 ();
rather than:
   ns3::foo_3 ();

[...snip...]

> --- gcc/testsuite/g++.dg/spellcheck-pr77829.C.jj	2018-10-31
> 10:31:10.213623350 +0100
> +++ gcc/testsuite/g++.dg/spellcheck-pr77829.C	2018-11-20
> 19:13:30.700008045 +0100
> @@ -21,8 +21,8 @@ void fn_1_explicit ()
>    detail::some_type i; // { dg-error ".some_type. is not a member of
> .detail.; did you mean 'some_typedef'\\?" }
>    /* { dg-begin-multiline-output "" }
>     detail::some_type i;
> -           ^~~~~~~~~
> -           some_typedef
> +   ~~~~~~~~^~~~~~~~~
> +   some_typedef
>       { dg-end-multiline-output "" } */
>  }

Similar problems here.
 
[...snip...]

So it looks like the less invasive fix might be better (not that I've
looked at it in detail, though).

Hope this is constructive
Dave

[C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 2)

[Jakub Jelinek]

Hi!

On Tue, Nov 20, 2018 at 04:32:26PM -0500, David Malcolm wrote:
> This makes the fix-it hint wrong: after the fix-it is applied, it will
> become
>   return color;
> (which won't compile), rather than
>   return O::color;
> which will.

Here is an updated version of the patch, which still uses the whole
range of the id-expression when it is parsed as primary expression, but does
so not in cp_parser_id_expression, but in cp_parser_primary_expression after
all the diagnostics.  Thus all the spell-checking etc. tests behave as
previously, they underline only the part after the last ::, and just
what uses the expression later on uses whole range.

The remaining needed tweeks in the testcases are minor and look correct to
me, e.g. for D::Bar the column is not at D but at B,
similarly for operator"" _F the column is under _ rather than first o.
The libstdc++ changes are because there are several large expressions like:
  something<one, two,
            three, four,
            five, six>::value
and we used to diagnose on the something line (column of s)
but now we warn on value line (column of v).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-11-21  Jakub Jelinek  <jakub@redhat.com>

	PR c++/87386
	* parser.c (cp_parser_primary_expression): Use
	id_expression.get_location () instead of id_expr_token->location.
	Adjust the range from id_expr_token->location to
	id_expressio.get_finish ().
	(cp_parser_operator): For operator "" make a range from "" to
	the end of the suffix with caret at the start of the id.
gcc/testsuite/
	* g++.dg/diagnostic/pr87386.C: New test.
	* g++.dg/parse/error17.C: Adjust expected diagnostics.
	* g++.dg/cpp0x/pr51420.C: Likewise.
	* g++.dg/cpp0x/udlit-declare-neg.C: Likewise.
	* g++.dg/cpp0x/udlit-member-neg.C: Likewise.
libstdc++-v3/
	* testsuite/20_util/scoped_allocator/69293_neg.cc: Adjust expected
	line.
	* testsuite/20_util/uses_allocator/cons_neg.cc: Likewise.
	* testsuite/20_util/uses_allocator/69293_neg.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements2.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements3.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements4.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements5.cc: Likewise.

--- gcc/cp/parser.c.jj	2018-11-21 11:35:43.698053550 +0100
+++ gcc/cp/parser.c	2018-11-21 12:23:20.701047164 +0100
@@ -5604,7 +5604,7 @@ cp_parser_primary_expression (cp_parser
 					  /*is_namespace=*/false,
 					  /*check_dependency=*/true,
 					  &ambiguous_decls,
-					  id_expr_token->location);
+					  id_expression.get_location ());
 	    /* If the lookup was ambiguous, an error will already have
 	       been issued.  */
 	    if (ambiguous_decls)
@@ -5675,7 +5675,7 @@ cp_parser_primary_expression (cp_parser
 	    if (parser->local_variables_forbidden_p
 		&& local_variable_p (decl))
 	      {
-		error_at (id_expr_token->location,
+		error_at (id_expression.get_location (),
 			  "local variable %qD may not appear in this context",
 			  decl.get_value ());
 		return error_mark_node;
@@ -5694,7 +5694,8 @@ cp_parser_primary_expression (cp_parser
 		 id_expression.get_location ()));
 	if (error_msg)
 	  cp_parser_error (parser, error_msg);
-	decl.set_location (id_expr_token->location);
+	decl.set_location (id_expression.get_location ());
+	decl.set_range (id_expr_token->location, id_expression.get_finish ());
 	return decl;
       }
 
@@ -15051,7 +15052,7 @@ cp_literal_operator_id (const char* name
 static cp_expr
 cp_parser_operator (cp_parser* parser)
 {
-  tree id = NULL_TREE;
+  cp_expr id = NULL_TREE;
   cp_token *token;
   bool utf8 = false;
 
@@ -15339,8 +15340,9 @@ cp_parser_operator (cp_parser* parser)
 	if (id != error_mark_node)
 	  {
 	    const char *name = IDENTIFIER_POINTER (id);
-	    id = cp_literal_operator_id (name);
+	    *id = cp_literal_operator_id (name);
 	  }
+	id.set_range (start_loc, id.get_finish ());
 	return id;
       }
 
@@ -15364,7 +15366,8 @@ cp_parser_operator (cp_parser* parser)
       id = error_mark_node;
     }
 
-  return cp_expr (id, start_loc);
+  id.set_location (start_loc);
+  return id;
 }
 
 /* Parse a template-declaration.
--- gcc/testsuite/g++.dg/diagnostic/pr87386.C.jj	2018-11-21 14:40:58.377769686 +0100
+++ gcc/testsuite/g++.dg/diagnostic/pr87386.C	2018-11-21 14:40:19.064410070 +0100
@@ -0,0 +1,18 @@
+// PR c++/87386
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdiagnostics-show-caret" }
+
+namespace foo {
+  template<typename> struct test { static constexpr bool value = false; };
+}
+static_assert (foo::test<int>::value, "foo");		// { dg-error "static assertion failed: foo" }
+/* { dg-begin-multiline-output "" }
+ static_assert (foo::test<int>::value, "foo");
+                ~~~~~~~~~~~~~~~~^~~~~
+   { dg-end-multiline-output "" } */
+
+static_assert (foo::test<int>::value && true, "bar");	// { dg-error "static assertion failed: bar" }
+/* { dg-begin-multiline-output "" }
+ static_assert (foo::test<int>::value && true, "bar");
+                ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
+   { dg-end-multiline-output "" } */
--- gcc/testsuite/g++.dg/parse/error17.C.jj	2018-11-20 21:39:01.844562232 +0100
+++ gcc/testsuite/g++.dg/parse/error17.C	2018-11-21 12:15:22.117967478 +0100
@@ -6,4 +6,4 @@ template <typename T> struct B {
 }; 
 struct D : B<int>, B<char> {}; 
  
-int i2 = D::Bar(2); // { dg-error "10:reference to 'Bar' is ambiguous" }
+int i2 = D::Bar(2); // { dg-error "13:reference to 'Bar' is ambiguous" }
--- gcc/testsuite/g++.dg/cpp0x/pr51420.C.jj	2018-11-20 21:39:02.000559703 +0100
+++ gcc/testsuite/g++.dg/cpp0x/pr51420.C	2018-11-21 12:15:22.117967478 +0100
@@ -3,6 +3,6 @@
 void
 foo()
 {
-  float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
+  float x = operator"" _F();  //  { dg-error  "24:'operator\"\"_F' was not declared in this scope" }
   float y = 0_F;  //  { dg-error  "unable to find numeric literal operator" }
 }
--- gcc/testsuite/g++.dg/cpp0x/udlit-member-neg.C.jj	2018-11-20 21:39:02.049558910 +0100
+++ gcc/testsuite/g++.dg/cpp0x/udlit-member-neg.C	2018-11-21 12:15:22.117967478 +0100
@@ -7,7 +7,7 @@ public:
   int operator"" _Bar(char32_t);  // { dg-error "7:.int Foo::operator\"\"_Bar\\(char32_t\\). must be a non-member function" }
 };
 
-int i = operator"" _Bar(U'x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
+int i = operator"" _Bar(U'x');  // { dg-error "20:'operator\"\"_Bar' was not declared in this scope" }
 int j = U'x'_Bar;  // { dg-error "unable to find character literal operator" }
 
 int
--- gcc/testsuite/g++.dg/cpp0x/udlit-declare-neg.C.jj	2018-11-20 21:39:01.949560531 +0100
+++ gcc/testsuite/g++.dg/cpp0x/udlit-declare-neg.C	2018-11-21 12:15:22.117967478 +0100
@@ -2,14 +2,14 @@
 
 //  Check that undeclared literal operator calls and literals give appropriate errors.
 
-int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
+int i = operator"" _Bar('x');  // { dg-error "20:'operator\"\"_Bar' was not declared in this scope" }
 int j = 'x'_Bar;  // { dg-error "unable to find character literal operator|with|argument" }
 
-int ii = operator"" _BarCharStr("Howdy, Pardner!");  // { dg-error "10:'operator\"\"_BarCharStr' was not declared in this scope" }
+int ii = operator"" _BarCharStr("Howdy, Pardner!");  // { dg-error "21:'operator\"\"_BarCharStr' was not declared in this scope" }
 int jj = "Howdy, Pardner!"_BarCharStr;  // { dg-error "unable to find string literal operator|Possible missing length argument" }
 
-unsigned long long iULL = operator"" _BarULL(666ULL);  // { dg-error "27:'operator\"\"_BarULL' was not declared in this scope" }
+unsigned long long iULL = operator"" _BarULL(666ULL);  // { dg-error "38:'operator\"\"_BarULL' was not declared in this scope" }
 unsigned long long jULL = 666_BarULL;  // { dg-error "unable to find numeric literal operator" }
 
-long double iLD = operator"" _BarLD(666.0L);  // { dg-error "19:'operator\"\"_BarLD' was not declared in this scope" }
+long double iLD = operator"" _BarLD(666.0L);  // { dg-error "30:'operator\"\"_BarLD' was not declared in this scope" }
 long double jLD = 666.0_BarLD;  // { dg-error "unable to find numeric literal operator" }
--- libstdc++-v3/testsuite/20_util/scoped_allocator/69293_neg.cc.jj	2018-07-27 22:20:29.000000000 +0200
+++ libstdc++-v3/testsuite/20_util/scoped_allocator/69293_neg.cc	2018-11-20 21:20:25.123636254 +0100
@@ -46,5 +46,5 @@ test01()
   scoped_alloc sa;
   auto p = sa.allocate(1);
   sa.construct(p);  // this is required to be ill-formed
-  // { dg-error "static assertion failed" "" { target *-*-* } 94 }
+  // { dg-error "static assertion failed" "" { target *-*-* } 96 }
 }
--- libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc.jj	2018-07-27 22:20:29.000000000 +0200
+++ libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc	2018-11-20 21:21:12.868864599 +0100
@@ -43,4 +43,4 @@ void test01()
 
   tuple<Type> t(allocator_arg, a, 1);
 }
-// { dg-error "static assertion failed" "" { target *-*-* } 94 }
+// { dg-error "static assertion failed" "" { target *-*-* } 96 }
--- libstdc++-v3/testsuite/20_util/uses_allocator/69293_neg.cc.jj	2018-07-27 22:20:29.760136498 +0200
+++ libstdc++-v3/testsuite/20_util/uses_allocator/69293_neg.cc	2018-11-21 16:47:49.563516993 +0100
@@ -44,5 +44,5 @@ test01()
 {
   alloc_type a;
   std::tuple<X> t(std::allocator_arg, a); // this is required to be ill-formed
-  // { dg-error "static assertion failed" "" { target *-*-* } 94 }
+  // { dg-error "static assertion failed" "" { target *-*-* } 96 }
 }
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements3.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements3.cc	2018-11-20 21:22:06.049005105 +0100
@@ -21,6 +21,6 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 
 propagate_const<void (*)()> test1;
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements4.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements4.cc	2018-11-20 21:22:29.698622883 +0100
@@ -21,7 +21,7 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 // { dg-error "invalid type" "" { target *-*-* } 66 }
 // { dg-error "uninitialized reference member" "" { target *-*-* } 112 }
 
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements5.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements5.cc	2018-11-20 21:22:45.311370550 +0100
@@ -21,6 +21,6 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 
 propagate_const<int[1]> test1;
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements2.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements2.cc	2018-11-20 21:21:43.959362116 +0100
@@ -21,7 +21,7 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 // { dg-error "not a pointer-to-object type" "" { target *-*-* } 66 }
 // { dg-error "forming pointer to reference type" "" { target *-*-* } 187 }
 // { dg-error "forming pointer to reference type" "" { target *-*-* } 213 }


	Jakub

Re: [C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 2)

[Jason Merrill]

On 11/21/18 10:55 AM, Jakub Jelinek wrote:
> Hi!
> 
> On Tue, Nov 20, 2018 at 04:32:26PM -0500, David Malcolm wrote:
>> This makes the fix-it hint wrong: after the fix-it is applied, it will
>> become
>>    return color;
>> (which won't compile), rather than
>>    return O::color;
>> which will.
> 
> Here is an updated version of the patch, which still uses the whole
> range of the id-expression when it is parsed as primary expression, but does
> so not in cp_parser_id_expression, but in cp_parser_primary_expression after
> all the diagnostics.  Thus all the spell-checking etc. tests behave as
> previously, they underline only the part after the last ::, and just
> what uses the expression later on uses whole range.
> 
> The remaining needed tweeks in the testcases are minor and look correct to
> me, e.g. for D::Bar the column is not at D but at B,

Sounds good.

> similarly for operator"" _F the column is under _ rather than first o.

I disagree with this one: the name of the declaration is operator""_F, 
so I think the caret should go at the first o.

> The libstdc++ changes are because there are several large expressions like:
>    something<one, two,
>              three, four,
>              five, six>::value
> and we used to diagnose on the something line (column of s)
> but now we warn on value line (column of v).

Makes sense.

Jason

Re: [C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 2)

[Jakub Jelinek]

On Wed, Nov 21, 2018 at 01:29:15PM -0500, Jason Merrill wrote:
> > similarly for operator"" _F the column is under _ rather than first o.
> 
> I disagree with this one: the name of the declaration is operator""_F, so I
> think the caret should go at the first o.

Right now when cp_parser_operator_function_id is called, it returns locus like:
operator new
         ^~~
operator delete []
         ^~~~~~~~~
operator ==
         ^
operator "" _foo
UNKNOWN_LOCATION
The last one is because for others we do return cp_expr (id, start_loc);
but for operator "" just return id;

So, do you suggest we should instead return
operator new
^~~~~~~~~~~~
operator delete []
^~~~~~~~~~~~~~~~~~
operator ==
^~~~~~~~~~~
operator "" _foo
^~~~~~~~~~~~~~~~
?
That would mean cp_parser_operator_function_id would need to pass
location_t start_loc (the start of the operator token) to cp_parser_operator and
let that create a range in all cases rather than just for operator
new/delete.

	Jakub

[C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 3)

[Jakub Jelinek]

On Wed, Nov 21, 2018 at 07:49:48PM +0100, Jakub Jelinek wrote:
> So, do you suggest we should instead return
> operator new
> ^~~~~~~~~~~~
> operator delete []
> ^~~~~~~~~~~~~~~~~~
> operator ==
> ^~~~~~~~~~~
> operator "" _foo
> ^~~~~~~~~~~~~~~~
> ?
> That would mean cp_parser_operator_function_id would need to pass
> location_t start_loc (the start of the operator token) to cp_parser_operator and
> let that create a range in all cases rather than just for operator
> new/delete.

This version of the patch implements that.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-11-21  Jakub Jelinek  <jakub@redhat.com>

	PR c++/87386
	* parser.c (cp_parser_primary_expression): Use
	id_expression.get_location () instead of id_expr_token->location.
	Adjust the range from id_expr_token->location to
	id_expressio.get_finish ().
	(cp_parser_operator_function_id): Pass location of the operator
	token down to cp_parser_operator.
	(cp_parser_operator): Add start_loc argument, always construct a
	location with caret at start_loc and range from start_loc to the
	finish of the last token.
gcc/testsuite/
	* g++.dg/diagnostic/pr87386.C: New test.
	* g++.dg/parse/error17.C: Adjust expected diagnostics.
libstdc++-v3/
	* testsuite/20_util/scoped_allocator/69293_neg.cc: Adjust expected
	line.
	* testsuite/20_util/uses_allocator/cons_neg.cc: Likewise.
	* testsuite/20_util/uses_allocator/69293_neg.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements2.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements3.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements4.cc: Likewise.
	* testsuite/experimental/propagate_const/requirements5.cc: Likewise.

--- gcc/cp/parser.c.jj	2018-11-21 17:42:18.003216049 +0100
+++ gcc/cp/parser.c	2018-11-21 20:56:43.694344258 +0100
@@ -2312,7 +2312,7 @@ static tree cp_parser_mem_initializer_id
 static cp_expr cp_parser_operator_function_id
   (cp_parser *);
 static cp_expr cp_parser_operator
-  (cp_parser *);
+  (cp_parser *, location_t);
 
 /* Templates [gram.temp] */
 
@@ -5604,7 +5604,7 @@ cp_parser_primary_expression (cp_parser
 					  /*is_namespace=*/false,
 					  /*check_dependency=*/true,
 					  &ambiguous_decls,
-					  id_expr_token->location);
+					  id_expression.get_location ());
 	    /* If the lookup was ambiguous, an error will already have
 	       been issued.  */
 	    if (ambiguous_decls)
@@ -5675,7 +5675,7 @@ cp_parser_primary_expression (cp_parser
 	    if (parser->local_variables_forbidden_p
 		&& local_variable_p (decl))
 	      {
-		error_at (id_expr_token->location,
+		error_at (id_expression.get_location (),
 			  "local variable %qD may not appear in this context",
 			  decl.get_value ());
 		return error_mark_node;
@@ -5694,7 +5694,8 @@ cp_parser_primary_expression (cp_parser
 		 id_expression.get_location ()));
 	if (error_msg)
 	  cp_parser_error (parser, error_msg);
-	decl.set_location (id_expr_token->location);
+	decl.set_location (id_expression.get_location ());
+	decl.set_range (id_expr_token->location, id_expression.get_finish ());
 	return decl;
       }
 
@@ -15011,11 +15012,12 @@ cp_parser_mem_initializer_id (cp_parser*
 static cp_expr
 cp_parser_operator_function_id (cp_parser* parser)
 {
+  location_t start_loc = cp_lexer_peek_token (parser->lexer)->location;
   /* Look for the `operator' keyword.  */
   if (!cp_parser_require_keyword (parser, RID_OPERATOR, RT_OPERATOR))
     return error_mark_node;
   /* And then the name of the operator itself.  */
-  return cp_parser_operator (parser);
+  return cp_parser_operator (parser, start_loc);
 }
 
 /* Return an identifier node for a user-defined literal operator.
@@ -15049,7 +15051,7 @@ cp_literal_operator_id (const char* name
    human-readable spelling of the identifier, e.g., `operator +'.  */
 
 static cp_expr
-cp_parser_operator (cp_parser* parser)
+cp_parser_operator (cp_parser* parser, location_t start_loc)
 {
   tree id = NULL_TREE;
   cp_token *token;
@@ -15058,7 +15060,7 @@ cp_parser_operator (cp_parser* parser)
   /* Peek at the next token.  */
   token = cp_lexer_peek_token (parser->lexer);
 
-  location_t start_loc = token->location;
+  location_t end_loc = token->location;
 
   /* Figure out which operator we have.  */
   enum tree_code op = ERROR_MARK;
@@ -15077,7 +15079,7 @@ cp_parser_operator (cp_parser* parser)
 	  break;
 
 	/* Consume the `new' or `delete' token.  */
-	location_t end_loc = cp_lexer_consume_token (parser->lexer)->location;
+	end_loc = cp_lexer_consume_token (parser->lexer)->location;
 
 	/* Peek at the next token.  */
 	token = cp_lexer_peek_token (parser->lexer);
@@ -15093,7 +15095,6 @@ cp_parser_operator (cp_parser* parser)
 	      end_loc = close_token->location;
 	    op = op == NEW_EXPR ? VEC_NEW_EXPR : VEC_DELETE_EXPR;
 	  }
-	start_loc = make_location (start_loc, start_loc, end_loc);
 	consumed = true;
 	break;
       }
@@ -15259,7 +15260,9 @@ cp_parser_operator (cp_parser* parser)
         matching_parens parens;
         parens.consume_open (parser);
         /* Look for the matching `)'.  */
-        parens.require_close (parser);
+        token = parens.require_close (parser);
+        if (token)
+	  end_loc = token->location;
 	op = CALL_EXPR;
 	consumed = true;
 	break;
@@ -15269,7 +15272,9 @@ cp_parser_operator (cp_parser* parser)
       /* Consume the `['.  */
       cp_lexer_consume_token (parser->lexer);
       /* Look for the matching `]'.  */
-      cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
+      token = cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
+      if (token)
+	end_loc = token->location;
       op = ARRAY_REF;
       consumed = true;
       break;
@@ -15287,7 +15292,8 @@ cp_parser_operator (cp_parser* parser)
     case CPP_STRING16_USERDEF:
     case CPP_STRING32_USERDEF:
       {
-	tree str, string_tree;
+	cp_expr str;
+	tree string_tree;
 	int sz, len;
 
 	if (cxx_dialect == cxx98)
@@ -15302,6 +15308,7 @@ cp_parser_operator (cp_parser* parser)
 	  {
 	    string_tree = USERDEF_LITERAL_VALUE (str);
 	    id = USERDEF_LITERAL_SUFFIX_ID (str);
+	    end_loc = str.get_location ();
 	  }
 	else
 	  {
@@ -15309,7 +15316,10 @@ cp_parser_operator (cp_parser* parser)
 	    /* Look for the suffix identifier.  */
 	    token = cp_lexer_peek_token (parser->lexer);
 	    if (token->type == CPP_NAME)
-	      id = cp_parser_identifier (parser);
+	      {
+		id = cp_parser_identifier (parser);
+		end_loc = token->location;
+	      }
 	    else if (token->type == CPP_KEYWORD)
 	      {
 		error ("unexpected keyword;"
@@ -15341,7 +15351,8 @@ cp_parser_operator (cp_parser* parser)
 	    const char *name = IDENTIFIER_POINTER (id);
 	    id = cp_literal_operator_id (name);
 	  }
-	return id;
+	start_loc = make_location (start_loc, start_loc, get_finish (end_loc));
+	return cp_expr (id, start_loc);
       }
 
     default:
@@ -15364,6 +15375,7 @@ cp_parser_operator (cp_parser* parser)
       id = error_mark_node;
     }
 
+  start_loc = make_location (start_loc, start_loc, get_finish (end_loc));
   return cp_expr (id, start_loc);
 }
 
--- gcc/testsuite/g++.dg/diagnostic/pr87386.C.jj	2018-11-21 20:17:22.707086226 +0100
+++ gcc/testsuite/g++.dg/diagnostic/pr87386.C	2018-11-21 20:17:22.707086226 +0100
@@ -0,0 +1,18 @@
+// PR c++/87386
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdiagnostics-show-caret" }
+
+namespace foo {
+  template<typename> struct test { static constexpr bool value = false; };
+}
+static_assert (foo::test<int>::value, "foo");		// { dg-error "static assertion failed: foo" }
+/* { dg-begin-multiline-output "" }
+ static_assert (foo::test<int>::value, "foo");
+                ~~~~~~~~~~~~~~~~^~~~~
+   { dg-end-multiline-output "" } */
+
+static_assert (foo::test<int>::value && true, "bar");	// { dg-error "static assertion failed: bar" }
+/* { dg-begin-multiline-output "" }
+ static_assert (foo::test<int>::value && true, "bar");
+                ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
+   { dg-end-multiline-output "" } */
--- gcc/testsuite/g++.dg/parse/error17.C.jj	2018-11-21 17:39:49.225651053 +0100
+++ gcc/testsuite/g++.dg/parse/error17.C	2018-11-21 20:17:22.707086226 +0100
@@ -6,4 +6,4 @@ template <typename T> struct B {
 }; 
 struct D : B<int>, B<char> {}; 
  
-int i2 = D::Bar(2); // { dg-error "10:reference to 'Bar' is ambiguous" }
+int i2 = D::Bar(2); // { dg-error "13:reference to 'Bar' is ambiguous" }
--- libstdc++-v3/testsuite/20_util/scoped_allocator/69293_neg.cc.jj	2018-07-27 22:20:29.000000000 +0200
+++ libstdc++-v3/testsuite/20_util/scoped_allocator/69293_neg.cc	2018-11-20 21:20:25.123636254 +0100
@@ -46,5 +46,5 @@ test01()
   scoped_alloc sa;
   auto p = sa.allocate(1);
   sa.construct(p);  // this is required to be ill-formed
-  // { dg-error "static assertion failed" "" { target *-*-* } 94 }
+  // { dg-error "static assertion failed" "" { target *-*-* } 96 }
 }
--- libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc.jj	2018-07-27 22:20:29.000000000 +0200
+++ libstdc++-v3/testsuite/20_util/uses_allocator/cons_neg.cc	2018-11-20 21:21:12.868864599 +0100
@@ -43,4 +43,4 @@ void test01()
 
   tuple<Type> t(allocator_arg, a, 1);
 }
-// { dg-error "static assertion failed" "" { target *-*-* } 94 }
+// { dg-error "static assertion failed" "" { target *-*-* } 96 }
--- libstdc++-v3/testsuite/20_util/uses_allocator/69293_neg.cc.jj	2018-07-27 22:20:29.760136498 +0200
+++ libstdc++-v3/testsuite/20_util/uses_allocator/69293_neg.cc	2018-11-21 16:47:49.563516993 +0100
@@ -44,5 +44,5 @@ test01()
 {
   alloc_type a;
   std::tuple<X> t(std::allocator_arg, a); // this is required to be ill-formed
-  // { dg-error "static assertion failed" "" { target *-*-* } 94 }
+  // { dg-error "static assertion failed" "" { target *-*-* } 96 }
 }
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements3.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements3.cc	2018-11-20 21:22:06.049005105 +0100
@@ -21,6 +21,6 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 
 propagate_const<void (*)()> test1;
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements4.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements4.cc	2018-11-20 21:22:29.698622883 +0100
@@ -21,7 +21,7 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 // { dg-error "invalid type" "" { target *-*-* } 66 }
 // { dg-error "uninitialized reference member" "" { target *-*-* } 112 }
 
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements5.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements5.cc	2018-11-20 21:22:45.311370550 +0100
@@ -21,6 +21,6 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 
 propagate_const<int[1]> test1;
--- libstdc++-v3/testsuite/experimental/propagate_const/requirements2.cc.jj	2018-01-03 18:53:16.000000000 +0100
+++ libstdc++-v3/testsuite/experimental/propagate_const/requirements2.cc	2018-11-20 21:21:43.959362116 +0100
@@ -21,7 +21,7 @@
 
 using std::experimental::propagate_const;
 
-// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 105 }
+// { dg-error "requires a class or a pointer to an object type" "" { target *-*-* } 107 }
 // { dg-error "not a pointer-to-object type" "" { target *-*-* } 66 }
 // { dg-error "forming pointer to reference type" "" { target *-*-* } 187 }
 // { dg-error "forming pointer to reference type" "" { target *-*-* } 213 }


	Jakub

Re: [C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 2)

[Jason Merrill]

On 11/21/18 1:49 PM, Jakub Jelinek wrote:
> On Wed, Nov 21, 2018 at 01:29:15PM -0500, Jason Merrill wrote:
>>> similarly for operator"" _F the column is under _ rather than first o.
>>
>> I disagree with this one: the name of the declaration is operator""_F, so I
>> think the caret should go at the first o.
> 
> Right now when cp_parser_operator_function_id is called, it returns locus like:
> operator new
>           ^~~
> operator delete []
>           ^~~~~~~~~
> operator ==
>           ^
> operator "" _foo
> UNKNOWN_LOCATION
> The last one is because for others we do return cp_expr (id, start_loc);
> but for operator "" just return id;
> 
> So, do you suggest we should instead return
> operator new
> ^~~~~~~~~~~~
> operator delete []
> ^~~~~~~~~~~~~~~~~~
> operator ==
> ^~~~~~~~~~~
> operator "" _foo
> ^~~~~~~~~~~~~~~~
> ?

Yes.

> That would mean cp_parser_operator_function_id would need to pass
> location_t start_loc (the start of the operator token) to cp_parser_operator and
> let that create a range in all cases rather than just for operator
> new/delete.

Sure.

Jason

Re: [C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 3)

[Jason Merrill]

On 11/21/18 5:10 PM, Jakub Jelinek wrote:
> On Wed, Nov 21, 2018 at 07:49:48PM +0100, Jakub Jelinek wrote:
>> So, do you suggest we should instead return
>> operator new
>> ^~~~~~~~~~~~
>> operator delete []
>> ^~~~~~~~~~~~~~~~~~
>> operator ==
>> ^~~~~~~~~~~
>> operator "" _foo
>> ^~~~~~~~~~~~~~~~
>> ?
>> That would mean cp_parser_operator_function_id would need to pass
>> location_t start_loc (the start of the operator token) to cp_parser_operator and
>> let that create a range in all cases rather than just for operator
>> new/delete.
> 
> This version of the patch implements that.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Jason

Re: [C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 3)

[Andreas Schwab]

In file included from ../../gcc/c-family/c-common.h:26:0,
                 from ../../gcc/cp/cp-tree.h:40,
                 from ../../gcc/cp/parser.c:25:
../../gcc/cp/parser.c: In function 'cp_expr cp_parser_operator(cp_parser*, location_t)':
../../gcc/tree.h:383:33: error: invalid cast from type 'cp_expr' to type 'tree_userdef_literal*'
 #define TREE_CHECK(T, CODE)   (T)
                                 ^
./tree-check.h:234:34: note: in expansion of macro 'TREE_CHECK'
 #define USERDEF_LITERAL_CHECK(t) TREE_CHECK (t, USERDEF_LITERAL)
                                  ^
../../gcc/c-family/c-common.h:1231:36: note: in expansion of macro 'USERDEF_LITERAL_CHECK'
   (((struct tree_userdef_literal *)USERDEF_LITERAL_CHECK (NODE))->value)
                                    ^
../../gcc/cp/parser.c:15309:20: note: in expansion of macro 'USERDEF_LITERAL_VALUE'
      string_tree = USERDEF_LITERAL_VALUE (str);
                    ^
../../gcc/tree.h:383:33: error: invalid cast from type 'cp_expr' to type 'tree_userdef_literal*'
 #define TREE_CHECK(T, CODE)   (T)
                                 ^
./tree-check.h:234:34: note: in expansion of macro 'TREE_CHECK'
 #define USERDEF_LITERAL_CHECK(t) TREE_CHECK (t, USERDEF_LITERAL)
                                  ^
../../gcc/c-family/c-common.h:1228:36: note: in expansion of macro 'USERDEF_LITERAL_CHECK'
   (((struct tree_userdef_literal *)USERDEF_LITERAL_CHECK (NODE))->suffix_id)
                                    ^
../../gcc/cp/parser.c:15310:11: note: in expansion of macro 'USERDEF_LITERAL_SUFFIX_ID'
      id = USERDEF_LITERAL_SUFFIX_ID (str);
           ^

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 3)

[Jakub Jelinek]

On Thu, Nov 22, 2018 at 09:53:13AM +0100, Andreas Schwab wrote:
> In file included from ../../gcc/c-family/c-common.h:26:0,
>                  from ../../gcc/cp/cp-tree.h:40,
>                  from ../../gcc/cp/parser.c:25:
> ../../gcc/cp/parser.c: In function 'cp_expr cp_parser_operator(cp_parser*, location_t)':
> ../../gcc/tree.h:383:33: error: invalid cast from type 'cp_expr' to type 'tree_userdef_literal*'
>  #define TREE_CHECK(T, CODE)   (T)
>                                  ^

Is that with --enable-checking=release or something similar?
Built just fine for me with normal --enable-checking=yes,rtl,extra.

	Jakub

Re: [C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 3)

[Andreas Schwab]

On Nov 22 2018, Jakub Jelinek <jakub@redhat.com> wrote:

> Is that with --enable-checking=release

Yes.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [C++ PATCH] Improve locations of id-expressions and operator "" (PR c++/87386, take 3)

[Jakub Jelinek]

On Thu, Nov 22, 2018 at 10:21:53AM +0100, Andreas Schwab wrote:
> On Nov 22 2018, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> > Is that with --enable-checking=release
> 
> Yes.

Ok, reproduced myself, fixed thusly, committed to unbreak it.

cp_expr has
  operator tree () const { return m_value; }
  tree & operator* () { return m_value; }
  tree operator* () const { return m_value; }
  tree & operator-> () { return m_value; }
  tree operator-> () const { return m_value; }
so it does the right thing most of the time, like with:
TREE_CHECK (NODE, something)->something_else
even if #define TREE_CHECK(NODE) NODE
But in this case we have:
#define USERDEF_LITERAL_VALUE(NODE) \
  (((struct tree_userdef_literal *)USERDEF_LITERAL_CHECK (NODE))->value)
and so it works only if TREE_CHECK expands to something that forces the cast
to tree.

2018-11-22  Jakub Jelinek  <jakub@redhat.com>

	PR c++/87386
	* parser.c (cp_parser_operator): Use str.get_value () instead of just
	str in USERDEF_LITERAL_VALUE and USERDEF_LITERAL_SUFFIX_ID arguments.

--- gcc/cp/parser.c.jj	2018-11-21 23:40:10.999140630 +0100
+++ gcc/cp/parser.c	2018-11-22 10:19:47.384240264 +0100
@@ -15306,8 +15306,8 @@ cp_parser_operator (cp_parser* parser, l
 	  return error_mark_node;
 	else if (TREE_CODE (str) == USERDEF_LITERAL)
 	  {
-	    string_tree = USERDEF_LITERAL_VALUE (str);
-	    id = USERDEF_LITERAL_SUFFIX_ID (str);
+	    string_tree = USERDEF_LITERAL_VALUE (str.get_value ());
+	    id = USERDEF_LITERAL_SUFFIX_ID (str.get_value ());
 	    end_loc = str.get_location ();
 	  }
 	else


	Jakub