Re: C++ PATCH for c++/91678 - wrong error with decltype and location wrapper

2019-12-06 Thread Jason Merrill 

On 12/6/19 7:18 PM, Marek Polacek wrote:

[ Sorry for dropping the ball on this. ]

On Tue, Sep 17, 2019 at 11:59:02PM -0400, Jason Merrill wrote:

On 9/16/19 1:12 PM, Marek Polacek wrote:

On Sun, Sep 15, 2019 at 10:18:29AM -0400, Jason Merrill wrote:

On 9/5/19 9:24 PM, Marek Polacek wrote:

They use
non_lvalue_loc, but that won't create a NON_LVALUE_EXPR wrapper around a 
location
wrapper.


That seems like the bug. maybe_lvalue_p should be true for
VIEW_CONVERT_EXPR.


That makes sense but it breaks in tsubst_* which doesn't expect a
NON_LVALUE_EXPR wrapped around a location wrapper.


Hmm, why would we get that in a template when we don't get NON_LVALUE_EXPR
wrapped around other lvalue nodes?


I just retested the patch without the pt.c hunk and no longer see that problem,
and the fold-const.c hunk still fixes the bogus error.  Yay?

So I think the following should be good to go:

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


OK.


2019-12-06  Marek Polacek  

PR c++/91678 - wrong error with decltype and location wrapper.
* fold-const.c (maybe_lvalue_p): Handle VIEW_CONVERT_EXPR.

* g++.dg/cpp0x/decltype73.C: New test.

--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -2594,6 +2594,7 @@ maybe_lvalue_p (const_tree x)
case TARGET_EXPR:
case COND_EXPR:
case BIND_EXPR:
+  case VIEW_CONVERT_EXPR:
  break;
  
default:

--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/decltype73.C
@@ -0,0 +1,4 @@
+// PR c++/91678 - wrong error with decltype and location wrapper.
+// { dg-do compile { target c++11 } }
+
+float* test(float* c) { return (decltype(c + 0))(float*)c; }

Re: C++ PATCH for c++/91678 - wrong error with decltype and location wrapper

2019-12-06 Thread Marek Polacek 
[ Sorry for dropping the ball on this. ]

On Tue, Sep 17, 2019 at 11:59:02PM -0400, Jason Merrill wrote:
> On 9/16/19 1:12 PM, Marek Polacek wrote:
> > On Sun, Sep 15, 2019 at 10:18:29AM -0400, Jason Merrill wrote:
> > > On 9/5/19 9:24 PM, Marek Polacek wrote:
> > > > They use
> > > > non_lvalue_loc, but that won't create a NON_LVALUE_EXPR wrapper around 
> > > > a location
> > > > wrapper.
> > > 
> > > That seems like the bug. maybe_lvalue_p should be true for
> > > VIEW_CONVERT_EXPR.
> > 
> > That makes sense but it breaks in tsubst_* which doesn't expect a
> > NON_LVALUE_EXPR wrapped around a location wrapper.
> 
> Hmm, why would we get that in a template when we don't get NON_LVALUE_EXPR
> wrapped around other lvalue nodes?

I just retested the patch without the pt.c hunk and no longer see that problem,
and the fold-const.c hunk still fixes the bogus error.  Yay?

So I think the following should be good to go:

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

2019-12-06  Marek Polacek  

PR c++/91678 - wrong error with decltype and location wrapper.
* fold-const.c (maybe_lvalue_p): Handle VIEW_CONVERT_EXPR.

* g++.dg/cpp0x/decltype73.C: New test.

--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -2594,6 +2594,7 @@ maybe_lvalue_p (const_tree x)
   case TARGET_EXPR:
   case COND_EXPR:
   case BIND_EXPR:
+  case VIEW_CONVERT_EXPR:
 break;
 
   default:
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/decltype73.C
@@ -0,0 +1,4 @@
+// PR c++/91678 - wrong error with decltype and location wrapper.
+// { dg-do compile { target c++11 } }
+
+float* test(float* c) { return (decltype(c + 0))(float*)c; }

Re: C++ PATCH for c++/91678 - wrong error with decltype and location wrapper

2019-09-17 Thread Jason Merrill 

On 9/16/19 1:12 PM, Marek Polacek wrote:

On Sun, Sep 15, 2019 at 10:18:29AM -0400, Jason Merrill wrote:

On 9/5/19 9:24 PM, Marek Polacek wrote:

They use
non_lvalue_loc, but that won't create a NON_LVALUE_EXPR wrapper around a 
location
wrapper.


That seems like the bug. maybe_lvalue_p should be true for
VIEW_CONVERT_EXPR.


That makes sense but it breaks in tsubst_* which doesn't expect a
NON_LVALUE_EXPR wrapped around a location wrapper.


Hmm, why would we get that in a template when we don't get 
NON_LVALUE_EXPR wrapped around other lvalue nodes?


Jason

Re: C++ PATCH for c++/91678 - wrong error with decltype and location wrapper

2019-09-16 Thread Marek Polacek 
On Sun, Sep 15, 2019 at 10:18:29AM -0400, Jason Merrill wrote:
> On 9/5/19 9:24 PM, Marek Polacek wrote:
> > They use
> > non_lvalue_loc, but that won't create a NON_LVALUE_EXPR wrapper around a 
> > location
> > wrapper.
> 
> That seems like the bug. maybe_lvalue_p should be true for
> VIEW_CONVERT_EXPR.

That makes sense but it breaks in tsubst_* which doesn't expect a
NON_LVALUE_EXPR wrapped around a location wrapper.  Perhaps we want to handle
it like this.

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

2019-09-16  Marek Polacek  

PR c++/91678 - wrong error with decltype and location wrapper.
* pt.c (tsubst_copy): Handle NON_LVALUE_EXPRs wrapped around a location
wrapper. 

* fold-const.c (maybe_lvalue_p): Handle VIEW_CONVERT_EXPR.

* g++.dg/cpp0x/decltype73.C: New test.

diff --git gcc/cp/pt.c gcc/cp/pt.c
index 9de1b8fec97..4bd77ac2578 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -15786,8 +15786,16 @@ tsubst_copy (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
  return op;
}
}
- /* We shouldn't see any other uses of these in templates.  */
- gcc_unreachable ();
+ /* We can get a NON_LVALUE_EXPR wrapped around a location wrapper.  */
+ else if (code == NON_LVALUE_EXPR && location_wrapper_p (op))
+   {
+ tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
+ op = tsubst_copy (TREE_OPERAND (t, 0), args, complain, in_decl);
+ return build1 (code, type, op);
+   }
+ else
+   /* We shouldn't see any other uses of these in templates.  */
+   gcc_unreachable ();
}
 
 case CAST_EXPR:
diff --git gcc/fold-const.c gcc/fold-const.c
index a99dafec589..6d955a76f87 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -2594,6 +2594,7 @@ maybe_lvalue_p (const_tree x)
   case TARGET_EXPR:
   case COND_EXPR:
   case BIND_EXPR:
+  case VIEW_CONVERT_EXPR:
 break;
 
   default:
diff --git gcc/testsuite/g++.dg/cpp0x/decltype73.C 
gcc/testsuite/g++.dg/cpp0x/decltype73.C
new file mode 100644
index 000..cbe94a898e3
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/decltype73.C
@@ -0,0 +1,4 @@
+// PR c++/91678 - wrong error with decltype and location wrapper.
+// { dg-do compile { target c++11 } }
+
+float* test(float* c) { return (decltype(c + 0))(float*)c; }

Re: C++ PATCH for c++/91678 - wrong error with decltype and location wrapper

2019-09-15 Thread Jason Merrill 

On 9/5/19 9:24 PM, Marek Polacek wrote:

They use
non_lvalue_loc, but that won't create a NON_LVALUE_EXPR wrapper around a 
location
wrapper.


That seems like the bug. maybe_lvalue_p should be true for 
VIEW_CONVERT_EXPR.


Jason

Re: C++ PATCH for c++/91678 - wrong error with decltype and location wrapper

2019-09-12 Thread Marek Polacek 
Ping.

On Thu, Sep 05, 2019 at 10:24:55PM -0400, Marek Polacek wrote:
> Compiling this testcase results in a bogus "invalid cast" error; this occurs
> since the introduction of location wrappers in finish_id_expression.
> 
> Here we are parsing the decltype expression via cp_parser_decltype_expr which
> can lead to calling various fold_* and c-family routines.  They use
> non_lvalue_loc, but that won't create a NON_LVALUE_EXPR wrapper around a 
> location
> wrapper.
> 
> So before the location wrappers addition cp_parser_decltype_expr would return
> NON_LVALUE_EXPR .  Now it returns VIEW_CONVERT_EXPR(c), but the
> STRIP_ANY_LOCATION_WRAPPER immediately following it strips the location 
> wrapper,
> and suddenly we don't know whether we have an lvalue anymore.  And that's sad
> because then decltype produces the wrong type, causing nonsense errors.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk and 9?
> 
> 2019-09-05  Marek Polacek  
> 
>   PR c++/91678 - wrong error with decltype and location wrapper.
>   * parser.c (cp_parser_decltype): Use auto_suppress_location_wrappers
>   sentinel.  Don't strip location wrappers.
> 
>   * g++.dg/cpp0x/decltype73.C: New test.
> 
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index baa60b8834e..b3c7bff5988 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -14729,8 +14729,13 @@ cp_parser_decltype (cp_parser *parser)
>/* Do not warn about problems with the expression.  */
>++c_inhibit_evaluation_warnings;
>  
> +  /* Don't create wrapper nodes within decltype.  non_lvalue_loc won't
> +  create a NON_LVALUE_EXPR wrapper around a location wrapper, and a
> +  subsequent STRIP_ANY_LOCATION_WRAPPER would destroy the information
> +  about lvalueness of the expression.  */
> +  auto_suppress_location_wrappers sentinel;
> +
>expr = cp_parser_decltype_expr (parser, 
> id_expression_or_member_access_p);
> -  STRIP_ANY_LOCATION_WRAPPER (expr);
>  
>/* Go back to evaluating expressions.  */
>--cp_unevaluated_operand;
> diff --git gcc/testsuite/g++.dg/cpp0x/decltype73.C 
> gcc/testsuite/g++.dg/cpp0x/decltype73.C
> new file mode 100644
> index 000..cbe94a898e3
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp0x/decltype73.C
> @@ -0,0 +1,4 @@
> +// PR c++/91678 - wrong error with decltype and location wrapper.
> +// { dg-do compile { target c++11 } }
> +
> +float* test(float* c) { return (decltype(c + 0))(float*)c; }