diff mbox

[C++] Improve finish_pseudo_destructor_expr location

Message ID 5230C454.7060202@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Sept. 11, 2013, 7:28 p.m. UTC 
Hi,

when yesterday I analyzed a bit c++/58363 and eventually I committed a 
pretty printing fix I noticed that the column was wrong for the pseudo 
destructor expression m.~f, pointing at the end. A fix turns out to be 
rather simple, because finish_pseudo_destructor_expr was simply not 
getting a location_t argument, I think it's also rather complete vs 
templates, otherwise it would not do the right thing for c++/58363 
itself for example, when the error message is produced by 
unify_arg_conversion.

Note, in tsubst_copy_and_build I simply pass input_location, not 
EXPR_LOCATION (t), which would cause a regression for the error @ line 
22 of pseudodtor3.C (because the location of t is UNKNOWN at that point 
during substitution whereas in fact the final location of the error 
messages was already Ok), neither EXPR_LOC_OR_HERE, which would buy 
nothing because tsubst_copy_and_build at the beginning has code which 
assigns input_location the EXPR_LOCATION (t), in case it's known.

Tested x86_64-linux.

Thanks!
Paolo.

////////////////////////////
2013-09-11  Paolo Carlini  <paolo.carlini@oracle.com>

	* semantics.c (finish_pseudo_destructor_expr): Add location_t
	parameter.
	* pt.c (unify_arg_conversion): Use EXPR_LOC_OR_HERE.
	(tsubst_copy_and_build): Adjust finish_pseudo_destructor_expr
	calls.
	* parser.c (cp_parser_postfix_dot_deref_expression): Likewise.
	(cp_parser_postfix_expression): Pass the proper location to
	cp_parser_postfix_dot_deref_expression.

/testsuite
2013-09-11  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/template/pseudodtor2.C: Add column number to dg-error
	strings.
	* g++.dg/template/pseudodtor3.C: Likewise.

Comments

Jason Merrill Sept. 12, 2013, 12:13 p.m. UTC | #1 
OK.

Jason
Richard Biener Sept. 12, 2013, 2:12 p.m. UTC | #2 
On Wed, Sep 11, 2013 at 9:28 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> when yesterday I analyzed a bit c++/58363 and eventually I committed a
> pretty printing fix I noticed that the column was wrong for the pseudo
> destructor expression m.~f, pointing at the end. A fix turns out to be
> rather simple, because finish_pseudo_destructor_expr was simply not getting
> a location_t argument, I think it's also rather complete vs templates,
> otherwise it would not do the right thing for c++/58363 itself for example,
> when the error message is produced by unify_arg_conversion.
>
> Note, in tsubst_copy_and_build I simply pass input_location, not
> EXPR_LOCATION (t), which would cause a regression for the error @ line 22 of
> pseudodtor3.C (because the location of t is UNKNOWN at that point during
> substitution whereas in fact the final location of the error messages was
> already Ok), neither EXPR_LOC_OR_HERE, which would buy nothing because
> tsubst_copy_and_build at the beginning has code which assigns input_location
> the EXPR_LOCATION (t), in case it's known.
>
> Tested x86_64-linux.

cp-tree.h misisng in commit and changelog.

Richard.

> Thanks!
> Paolo.
>
> ////////////////////////////
Paolo Carlini Sept. 12, 2013, 2:17 p.m. UTC | #3 
On 09/12/2013 04:12 PM, Richard Biener wrote:
> On Wed, Sep 11, 2013 at 9:28 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Hi,
>>
>> when yesterday I analyzed a bit c++/58363 and eventually I committed a
>> pretty printing fix I noticed that the column was wrong for the pseudo
>> destructor expression m.~f, pointing at the end. A fix turns out to be
>> rather simple, because finish_pseudo_destructor_expr was simply not getting
>> a location_t argument, I think it's also rather complete vs templates,
>> otherwise it would not do the right thing for c++/58363 itself for example,
>> when the error message is produced by unify_arg_conversion.
>>
>> Note, in tsubst_copy_and_build I simply pass input_location, not
>> EXPR_LOCATION (t), which would cause a regression for the error @ line 22 of
>> pseudodtor3.C (because the location of t is UNKNOWN at that point during
>> substitution whereas in fact the final location of the error messages was
>> already Ok), neither EXPR_LOC_OR_HERE, which would buy nothing because
>> tsubst_copy_and_build at the beginning has code which assigns input_location
>> the EXPR_LOCATION (t), in case it's known.
>>
>> Tested x86_64-linux.
> cp-tree.h misisng in commit and changelog.
>
> Richard.
Argh, sorry, will fix momentarily.

Paolo.
diff mbox

Patch

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 202503)
+++ cp/cp-tree.h	(working copy)
@@ -5734,7 +5734,7 @@  extern tree finish_call_expr			(tree, vec<tree, va
 						 bool, tsubst_flags_t);
 extern tree finish_increment_expr		(tree, enum tree_code);
 extern tree finish_this_expr			(void);
-extern tree finish_pseudo_destructor_expr       (tree, tree, tree);
+extern tree finish_pseudo_destructor_expr       (tree, tree, tree, location_t);
 extern tree finish_unary_op_expr		(location_t, enum tree_code, tree,
 						 tsubst_flags_t);
 extern tree finish_compound_literal		(tree, tree, tsubst_flags_t);
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 202503)
+++ cp/parser.c	(working copy)
@@ -5533,6 +5533,7 @@  cp_parser_postfix_expression (cp_parser *parser, b
 			      cp_id_kind * pidk_return)
 {
   cp_token *token;
+  location_t sloc;
   enum rid keyword;
   cp_id_kind idk = CP_ID_KIND_NONE;
   tree postfix_expression = NULL_TREE;
@@ -5540,6 +5541,7 @@  cp_parser_postfix_expression (cp_parser *parser, b
 
   /* Peek at the next token.  */
   token = cp_lexer_peek_token (parser->lexer);
+  sloc = token->location;
   /* Some of the productions are determined by keywords.  */
   keyword = token->keyword;
   switch (keyword)
@@ -6019,7 +6021,7 @@  cp_parser_postfix_expression (cp_parser *parser, b
 	    = cp_parser_postfix_dot_deref_expression (parser, token->type,
 						      postfix_expression,
 						      false, &idk,
-						      token->location);
+						      sloc);
 
           is_member_access = true;
 	  break;
@@ -6338,7 +6340,7 @@  cp_parser_postfix_dot_deref_expression (cp_parser
 	  pseudo_destructor_p = true;
 	  postfix_expression
 	    = finish_pseudo_destructor_expr (postfix_expression,
-					     s, type);
+					     s, type, location);
 	}
     }
 
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 202503)
+++ cp/pt.c	(working copy)
@@ -5398,7 +5398,8 @@  unify_arg_conversion (bool explain_p, tree to_type
 		      tree from_type, tree arg)
 {
   if (explain_p)
-    inform (input_location, "  cannot convert %qE (type %qT) to type %qT",
+    inform (EXPR_LOC_OR_HERE (arg),
+	    "  cannot convert %qE (type %qT) to type %qT",
 	    arg, from_type, to_type);
   return 1;
 }
@@ -14292,9 +14293,10 @@  tsubst_copy_and_build (tree t,
 
     case PSEUDO_DTOR_EXPR:
       RETURN (finish_pseudo_destructor_expr
-	(RECUR (TREE_OPERAND (t, 0)),
-	 RECUR (TREE_OPERAND (t, 1)),
-	 tsubst (TREE_OPERAND (t, 2), args, complain, in_decl)));
+	      (RECUR (TREE_OPERAND (t, 0)),
+	       RECUR (TREE_OPERAND (t, 1)),
+	       tsubst (TREE_OPERAND (t, 2), args, complain, in_decl),
+	       input_location));
 
     case TREE_LIST:
       {
@@ -14423,7 +14425,8 @@  tsubst_copy_and_build (tree t,
 		  {
 		    dtor = TREE_OPERAND (dtor, 0);
 		    if (TYPE_P (dtor))
-		      RETURN (finish_pseudo_destructor_expr (object, s, dtor));
+		      RETURN (finish_pseudo_destructor_expr
+			      (object, s, dtor, input_location));
 		  }
 	      }
 	  }
Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 202503)
+++ cp/semantics.c	(working copy)
@@ -2361,7 +2361,8 @@  finish_this_expr (void)
    was of the form `OBJECT.SCOPE::~DESTRUCTOR'.  */
 
 tree
-finish_pseudo_destructor_expr (tree object, tree scope, tree destructor)
+finish_pseudo_destructor_expr (tree object, tree scope, tree destructor,
+			       location_t loc)
 {
   if (object == error_mark_node || destructor == error_mark_node)
     return error_mark_node;
@@ -2372,15 +2373,16 @@  tree
     {
       if (scope == error_mark_node)
 	{
-	  error ("invalid qualifying scope in pseudo-destructor name");
+	  error_at (loc, "invalid qualifying scope in pseudo-destructor name");
 	  return error_mark_node;
 	}
       if (is_auto (destructor))
 	destructor = TREE_TYPE (object);
       if (scope && TYPE_P (scope) && !check_dtor_name (scope, destructor))
 	{
-	  error ("qualified type %qT does not match destructor name ~%qT",
-		 scope, destructor);
+	  error_at (loc,
+		    "qualified type %qT does not match destructor name ~%qT",
+		    scope, destructor);
 	  return error_mark_node;
 	}
 
@@ -2401,12 +2403,13 @@  tree
       if (!same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (object),
 						      destructor))
 	{
-	  error ("%qE is not of type %qT", object, destructor);
+	  error_at (loc, "%qE is not of type %qT", object, destructor);
 	  return error_mark_node;
 	}
     }
 
-  return build3 (PSEUDO_DTOR_EXPR, void_type_node, object, scope, destructor);
+  return build3_loc (loc, PSEUDO_DTOR_EXPR, void_type_node, object,
+		     scope, destructor);
 }
 
 /* Finish an expression of the form CODE EXPR.  */
Index: testsuite/g++.dg/template/pseudodtor2.C
===================================================================
--- testsuite/g++.dg/template/pseudodtor2.C	(revision 202503)
+++ testsuite/g++.dg/template/pseudodtor2.C	(working copy)
@@ -6,7 +6,7 @@  template<typename S> struct D
   typedef int T;
   S foo ();
 
-  D () { foo ().~T(); }		// { dg-error "is not of type" }
+  D () { foo ().~T(); }		// { dg-error "10:is not of type" }
 };
 
 struct Z
Index: testsuite/g++.dg/template/pseudodtor3.C
===================================================================
--- testsuite/g++.dg/template/pseudodtor3.C	(revision 202503)
+++ testsuite/g++.dg/template/pseudodtor3.C	(working copy)
@@ -5,13 +5,13 @@  struct A
 {
   typedef int T;
   T &foo ();
-  A () { foo.~T (); }	// { dg-error "does not have class type|expected" }
+  A () { foo.~T (); }	// { dg-error "10:does not have class type|expected" }
 };
 
 template <typename T> struct B
 {
   T &foo ();
-  B () { foo.~T (); }	// { dg-error "invalid use of member" }
+  B () { foo.~T (); }	// { dg-error "10:invalid use of member" }
 };
 
 B<int> b;
@@ -19,7 +19,7 @@  B<int> b;
 template <typename T, typename S> struct C
 {
   T t;
-  C () { t.~S (); }	// { dg-error "is not of type" }
+  C () { t.~S (); }	// { dg-error "10:is not of type" }
 };
 
 C<int, long int> c;
@@ -28,7 +28,7 @@  template <typename T> struct D
 {
   T t;
   typedef long int U;
-  D () { t.~U (); }	// { dg-error "is not of type" }
+  D () { t.~U (); }	// { dg-error "10:is not of type" }
 };
 
 D<int> d;
@@ -37,7 +37,7 @@  template <typename T> struct E
 {
   T &foo ();
   typedef long int U;
-  E () { foo.~U (); }	// { dg-error "is not of type" }
+  E () { foo.~U (); }	// { dg-error "10:is not of type" }
 };
 
 E<int> e;