diff mbox series

C++ PATCH for c++/91230 - wrong error with __PRETTY_FUNCTION__ and generic lambda

Message ID 20190801195050.GR32749@redhat.com
State New
Headers show
Series C++ PATCH for c++/91230 - wrong error with __PRETTY_FUNCTION__ and generic lambda | expand

Commit Message

Marek Polacek Aug. 1, 2019, 7:50 p.m. UTC 
We started rejecting this test with r268321, whereby we process the
compound_literal in finish_compound_literal normally even in a template
if it's not instantiation-dependent.  This broke with __PRETTY_FUNCTION__
in a template function, because instantiation_dependent_expression_p didn't
consider it dependent, which resulted in a bogus error later.

cp_make_fname_decl has TYPE_DEP indicating that the fname is dependent, so
I think value_dependent_expression_p needs to be fixed as below, and actually
treat such a fname as dependent.

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

2019-08-01  Marek Polacek  <polacek@redhat.com>

	PR c++/91230 - wrong error with __PRETTY_FUNCTION__ and generic lambda.
	* pt.c (value_dependent_expression_p): Consider __PRETTY_FUNCTION__
	inside a template function value-dependent.

	* g++.dg/cpp1y/lambda-generic-pretty1.C: New test.

Comments

Jason Merrill Aug. 1, 2019, 9:11 p.m. UTC | #1 
On 8/1/19 3:50 PM, Marek Polacek wrote:
> We started rejecting this test with r268321, whereby we process the
> compound_literal in finish_compound_literal normally even in a template
> if it's not instantiation-dependent.  This broke with __PRETTY_FUNCTION__
> in a template function, because instantiation_dependent_expression_p didn't
> consider it dependent, which resulted in a bogus error later.
> 
> cp_make_fname_decl has TYPE_DEP indicating that the fname is dependent, so
> I think value_dependent_expression_p needs to be fixed as below, and actually
> treat such a fname as dependent.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk and 9?
> 
> 2019-08-01  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c++/91230 - wrong error with __PRETTY_FUNCTION__ and generic lambda.
> 	* pt.c (value_dependent_expression_p): Consider __PRETTY_FUNCTION__
> 	inside a template function value-dependent.
> 
> 	* g++.dg/cpp1y/lambda-generic-pretty1.C: New test.
> 
> diff --git gcc/cp/pt.c gcc/cp/pt.c
> index 91a46745447..94706bc5ad1 100644
> --- gcc/cp/pt.c
> +++ gcc/cp/pt.c
> @@ -25553,7 +25553,11 @@ value_dependent_expression_p (tree expression)
>         if (DECL_HAS_VALUE_EXPR_P (expression))
>   	{
>   	  tree value_expr = DECL_VALUE_EXPR (expression);
> -	  if (value_dependent_expression_p (value_expr))
> +	  if (value_dependent_expression_p (value_expr)
> +	      /* __PRETTY_FUNCTION__ inside a template function is dependent
> +		 on the name of the function.  */
> +	      || (DECL_PRETTY_FUNCTION_P (expression)
> +		  && value_expr == error_mark_node))

Hmm, when would value_expr not be error_mark_node for 
__PRETTY_FUNCTION__ in a template?

Jason
Marek Polacek Aug. 1, 2019, 9:53 p.m. UTC | #2 
On Thu, Aug 01, 2019 at 05:11:47PM -0400, Jason Merrill wrote:
> On 8/1/19 3:50 PM, Marek Polacek wrote:
> > We started rejecting this test with r268321, whereby we process the
> > compound_literal in finish_compound_literal normally even in a template
> > if it's not instantiation-dependent.  This broke with __PRETTY_FUNCTION__
> > in a template function, because instantiation_dependent_expression_p didn't
> > consider it dependent, which resulted in a bogus error later.
> > 
> > cp_make_fname_decl has TYPE_DEP indicating that the fname is dependent, so
> > I think value_dependent_expression_p needs to be fixed as below, and actually
> > treat such a fname as dependent.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk and 9?
> > 
> > 2019-08-01  Marek Polacek  <polacek@redhat.com>
> > 
> > 	PR c++/91230 - wrong error with __PRETTY_FUNCTION__ and generic lambda.
> > 	* pt.c (value_dependent_expression_p): Consider __PRETTY_FUNCTION__
> > 	inside a template function value-dependent.
> > 
> > 	* g++.dg/cpp1y/lambda-generic-pretty1.C: New test.
> > 
> > diff --git gcc/cp/pt.c gcc/cp/pt.c
> > index 91a46745447..94706bc5ad1 100644
> > --- gcc/cp/pt.c
> > +++ gcc/cp/pt.c
> > @@ -25553,7 +25553,11 @@ value_dependent_expression_p (tree expression)
> >         if (DECL_HAS_VALUE_EXPR_P (expression))
> >   	{
> >   	  tree value_expr = DECL_VALUE_EXPR (expression);
> > -	  if (value_dependent_expression_p (value_expr))
> > +	  if (value_dependent_expression_p (value_expr)
> > +	      /* __PRETTY_FUNCTION__ inside a template function is dependent
> > +		 on the name of the function.  */
> > +	      || (DECL_PRETTY_FUNCTION_P (expression)
> > +		  && value_expr == error_mark_node))
> 
> Hmm, when would value_expr not be error_mark_node for __PRETTY_FUNCTION__ in
> a template?

E.g.

template<typename T>
struct S {
  T *t{__PRETTY_FUNCTION__};
};

S<const char> s;

here cp_make_fname_decl is called in a template, but in_template_function() is
false, so we create a decl whose DECL_VALUE_EXPR is "top level".

Marek
Jason Merrill Aug. 2, 2019, 1:20 a.m. UTC | #3 
On 8/1/19 5:53 PM, Marek Polacek wrote:
> On Thu, Aug 01, 2019 at 05:11:47PM -0400, Jason Merrill wrote:
>> On 8/1/19 3:50 PM, Marek Polacek wrote:
>>> We started rejecting this test with r268321, whereby we process the
>>> compound_literal in finish_compound_literal normally even in a template
>>> if it's not instantiation-dependent.  This broke with __PRETTY_FUNCTION__
>>> in a template function, because instantiation_dependent_expression_p didn't
>>> consider it dependent, which resulted in a bogus error later.
>>>
>>> cp_make_fname_decl has TYPE_DEP indicating that the fname is dependent, so
>>> I think value_dependent_expression_p needs to be fixed as below, and actually
>>> treat such a fname as dependent.
>>>
>>> Bootstrapped/regtested on x86_64-linux, ok for trunk and 9?
>>>
>>> 2019-08-01  Marek Polacek  <polacek@redhat.com>
>>>
>>> 	PR c++/91230 - wrong error with __PRETTY_FUNCTION__ and generic lambda.
>>> 	* pt.c (value_dependent_expression_p): Consider __PRETTY_FUNCTION__
>>> 	inside a template function value-dependent.
>>>
>>> 	* g++.dg/cpp1y/lambda-generic-pretty1.C: New test.
>>>
>>> diff --git gcc/cp/pt.c gcc/cp/pt.c
>>> index 91a46745447..94706bc5ad1 100644
>>> --- gcc/cp/pt.c
>>> +++ gcc/cp/pt.c
>>> @@ -25553,7 +25553,11 @@ value_dependent_expression_p (tree expression)
>>>          if (DECL_HAS_VALUE_EXPR_P (expression))
>>>    	{
>>>    	  tree value_expr = DECL_VALUE_EXPR (expression);
>>> -	  if (value_dependent_expression_p (value_expr))
>>> +	  if (value_dependent_expression_p (value_expr)
>>> +	      /* __PRETTY_FUNCTION__ inside a template function is dependent
>>> +		 on the name of the function.  */
>>> +	      || (DECL_PRETTY_FUNCTION_P (expression)
>>> +		  && value_expr == error_mark_node))
>>
>> Hmm, when would value_expr not be error_mark_node for __PRETTY_FUNCTION__ in
>> a template?
> 
> E.g.
> 
> template<typename T>
> struct S {
>    T *t{__PRETTY_FUNCTION__};
> };
> 
> S<const char> s;
> 
> here cp_make_fname_decl is called in a template, but in_template_function() is
> false, so we create a decl whose DECL_VALUE_EXPR is "top level".

Ah, sure.  The patch is OK, but you might mention that in the comment.

Jason
Marek Polacek Aug. 2, 2019, 1:24 p.m. UTC | #4 
On Thu, Aug 01, 2019 at 09:20:19PM -0400, Jason Merrill wrote:
> On 8/1/19 5:53 PM, Marek Polacek wrote:
> > > Hmm, when would value_expr not be error_mark_node for __PRETTY_FUNCTION__ in
> > > a template?
> > 
> > E.g.
> > 
> > template<typename T>
> > struct S {
> >    T *t{__PRETTY_FUNCTION__};
> > };
> > 
> > S<const char> s;
> > 
> > here cp_make_fname_decl is called in a template, but in_template_function() is
> > false, so we create a decl whose DECL_VALUE_EXPR is "top level".
> 
> Ah, sure.  The patch is OK, but you might mention that in the comment.

I've added a comment and committed.  Thanks.

Marek
diff mbox series

Patch

diff --git gcc/cp/pt.c gcc/cp/pt.c
index 91a46745447..94706bc5ad1 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -25553,7 +25553,11 @@  value_dependent_expression_p (tree expression)
       if (DECL_HAS_VALUE_EXPR_P (expression))
 	{
 	  tree value_expr = DECL_VALUE_EXPR (expression);
-	  if (value_dependent_expression_p (value_expr))
+	  if (value_dependent_expression_p (value_expr)
+	      /* __PRETTY_FUNCTION__ inside a template function is dependent
+		 on the name of the function.  */
+	      || (DECL_PRETTY_FUNCTION_P (expression)
+		  && value_expr == error_mark_node))
 	    return true;
 	}
       return false;
diff --git gcc/testsuite/g++.dg/cpp1y/lambda-generic-pretty1.C gcc/testsuite/g++.dg/cpp1y/lambda-generic-pretty1.C
new file mode 100644
index 00000000000..4d3246b909b
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1y/lambda-generic-pretty1.C
@@ -0,0 +1,17 @@ 
+// PR c++/91230
+// { dg-do compile { target c++14 } }
+
+struct StringWrapper {
+    const char* Value;
+};
+
+template <typename T>
+void f() {
+    [](auto) {
+        StringWrapper{__PRETTY_FUNCTION__};
+    };
+}
+
+int main() {
+    f<int>();
+}