| Message ID | 20210129210251.1824003-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Improve sorry for __builtin_has_attribute [PR98355] | expand |
On Fri, Jan 29, 2021 at 04:02:51PM -0500, Marek Polacek via Gcc-patches wrote: > __builtin_has_attribute doesn't work in templates yet (bug 92104), so > in r11-471 I added a sorry. But that only caught type-dependent > expressions and we also want to sorry on value-dependent expressions. > This patch uses v_d_e_p rather than uses_template_parms because u_t_p > sets p_t_d and then v_d_e_p considers variables with reference types > value-dependent, which breaks builtin-has-attribute-6.c. > > This is a regression and I also plan to apply this to gcc-10. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > gcc/cp/ChangeLog: > > PR c++/98355 > * parser.c (cp_parser_has_attribute_expression): Use > value_dependent_expression_p instead of type_dependent_expression_p. > > gcc/testsuite/ChangeLog: > > PR c++/98355 > * g++.dg/ext/builtin-has-attribute2.C: New test. > --- > gcc/cp/parser.c | 2 +- > gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 5c1d880c9fc..7b1dc0dc93f 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) > { > if (oper == error_mark_node) > /* Nothing. */; > - else if (type_dependent_expression_p (oper)) > + else if (value_dependent_expression_p (oper)) > sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " > "not supported yet"); Actually I don't like this. I think we want processing_template_decl && uses_template_parms () here. Marek
On Fri, Jan 29, 2021 at 04:23:14PM -0500, Marek Polacek via Gcc-patches wrote: > On Fri, Jan 29, 2021 at 04:02:51PM -0500, Marek Polacek via Gcc-patches wrote: > > __builtin_has_attribute doesn't work in templates yet (bug 92104), so > > in r11-471 I added a sorry. But that only caught type-dependent > > expressions and we also want to sorry on value-dependent expressions. > > This patch uses v_d_e_p rather than uses_template_parms because u_t_p > > sets p_t_d and then v_d_e_p considers variables with reference types > > value-dependent, which breaks builtin-has-attribute-6.c. > > > > This is a regression and I also plan to apply this to gcc-10. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > > > gcc/cp/ChangeLog: > > > > PR c++/98355 > > * parser.c (cp_parser_has_attribute_expression): Use > > value_dependent_expression_p instead of type_dependent_expression_p. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/98355 > > * g++.dg/ext/builtin-has-attribute2.C: New test. > > --- > > gcc/cp/parser.c | 2 +- > > gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ > > 2 files changed, 9 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C > > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > > index 5c1d880c9fc..7b1dc0dc93f 100644 > > --- a/gcc/cp/parser.c > > +++ b/gcc/cp/parser.c > > @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) > > { > > if (oper == error_mark_node) > > /* Nothing. */; > > - else if (type_dependent_expression_p (oper)) > > + else if (value_dependent_expression_p (oper)) > > sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " > > "not supported yet"); > > Actually I don't like this. I think we want > > processing_template_decl && uses_template_parms () > > here. So here's v2. Sorry for the self-review. -- >8 -- __builtin_has_attribute doesn't work in templates yet (bug 92104), so in r11-471 I added a sorry. But that only caught type-dependent expressions and we also want to sorry on value-dependent expressions. This patch uses uses_template_parms, but guarded with p_t_d, because u_t_p sets p_t_d and then v_d_e_p considers variables with reference types value-dependent, which breaks builtin-has-attribute-6.c. This is a regression and I also plan to apply this to gcc-10. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? gcc/cp/ChangeLog: PR c++/98355 * parser.c (cp_parser_has_attribute_expression): Use uses_template_parms instead of type_dependent_expression_p. gcc/testsuite/ChangeLog: PR c++/98355 * g++.dg/ext/builtin-has-attribute2.C: New test. --- gcc/cp/parser.c | 2 +- gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 5c1d880c9fc..abadaf972d6 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) { if (oper == error_mark_node) /* Nothing. */; - else if (type_dependent_expression_p (oper)) + else if (processing_template_decl && uses_template_parms (oper)) sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " "not supported yet"); else diff --git a/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C b/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C new file mode 100644 index 00000000000..aba7932a136 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C @@ -0,0 +1,8 @@ +// PR c++/98355 +// { dg-do compile { target c++11 } } + +struct S { int a; }; +template <int> struct T +{ + static_assert (!__builtin_has_attribute (((S*)0) -> a, packed), ""); // { dg-message "sorry, unimplemented: .__builtin_has_attribute. with dependent argument not supported yet" } +}; base-commit: 726b7aa004d6885388a76521222602b8552a41ee
On 1/29/21 5:52 PM, Marek Polacek wrote: > On Fri, Jan 29, 2021 at 04:23:14PM -0500, Marek Polacek via Gcc-patches wrote: >> On Fri, Jan 29, 2021 at 04:02:51PM -0500, Marek Polacek via Gcc-patches wrote: >>> __builtin_has_attribute doesn't work in templates yet (bug 92104), so >>> in r11-471 I added a sorry. But that only caught type-dependent >>> expressions and we also want to sorry on value-dependent expressions. >>> This patch uses v_d_e_p rather than uses_template_parms because u_t_p >>> sets p_t_d and then v_d_e_p considers variables with reference types >>> value-dependent, which breaks builtin-has-attribute-6.c. >>> >>> This is a regression and I also plan to apply this to gcc-10. >>> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >>> >>> gcc/cp/ChangeLog: >>> >>> PR c++/98355 >>> * parser.c (cp_parser_has_attribute_expression): Use >>> value_dependent_expression_p instead of type_dependent_expression_p. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/98355 >>> * g++.dg/ext/builtin-has-attribute2.C: New test. >>> --- >>> gcc/cp/parser.c | 2 +- >>> gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ >>> 2 files changed, 9 insertions(+), 1 deletion(-) >>> create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C >>> >>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c >>> index 5c1d880c9fc..7b1dc0dc93f 100644 >>> --- a/gcc/cp/parser.c >>> +++ b/gcc/cp/parser.c >>> @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) >>> { >>> if (oper == error_mark_node) >>> /* Nothing. */; >>> - else if (type_dependent_expression_p (oper)) >>> + else if (value_dependent_expression_p (oper)) >>> sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " >>> "not supported yet"); >> >> Actually I don't like this. I think we want >> >> processing_template_decl && uses_template_parms () >> >> here. > > So here's v2. Sorry for the self-review. > > -- >8 -- > __builtin_has_attribute doesn't work in templates yet (bug 92104), so > in r11-471 I added a sorry. But that only caught type-dependent > expressions and we also want to sorry on value-dependent expressions. > This patch uses uses_template_parms, but guarded with p_t_d, because > u_t_p sets p_t_d and then v_d_e_p considers variables with reference > types value-dependent, which breaks builtin-has-attribute-6.c. Maybe instantiation_dependent_expression_p? > This is a regression and I also plan to apply this to gcc-10. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > gcc/cp/ChangeLog: > > PR c++/98355 > * parser.c (cp_parser_has_attribute_expression): Use > uses_template_parms instead of type_dependent_expression_p. > > gcc/testsuite/ChangeLog: > > PR c++/98355 > * g++.dg/ext/builtin-has-attribute2.C: New test. > --- > gcc/cp/parser.c | 2 +- > gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 5c1d880c9fc..abadaf972d6 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) > { > if (oper == error_mark_node) > /* Nothing. */; > - else if (type_dependent_expression_p (oper)) > + else if (processing_template_decl && uses_template_parms (oper)) > sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " > "not supported yet"); > else > diff --git a/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C b/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C > new file mode 100644 > index 00000000000..aba7932a136 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C > @@ -0,0 +1,8 @@ > +// PR c++/98355 > +// { dg-do compile { target c++11 } } > + > +struct S { int a; }; > +template <int> struct T > +{ > + static_assert (!__builtin_has_attribute (((S*)0) -> a, packed), ""); // { dg-message "sorry, unimplemented: .__builtin_has_attribute. with dependent argument not supported yet" } > +}; > > base-commit: 726b7aa004d6885388a76521222602b8552a41ee >
On Fri, Jan 29, 2021 at 06:18:35PM -0500, Jason Merrill via Gcc-patches wrote: > On 1/29/21 5:52 PM, Marek Polacek wrote: > > On Fri, Jan 29, 2021 at 04:23:14PM -0500, Marek Polacek via Gcc-patches wrote: > > > On Fri, Jan 29, 2021 at 04:02:51PM -0500, Marek Polacek via Gcc-patches wrote: > > > > __builtin_has_attribute doesn't work in templates yet (bug 92104), so > > > > in r11-471 I added a sorry. But that only caught type-dependent > > > > expressions and we also want to sorry on value-dependent expressions. > > > > This patch uses v_d_e_p rather than uses_template_parms because u_t_p > > > > sets p_t_d and then v_d_e_p considers variables with reference types > > > > value-dependent, which breaks builtin-has-attribute-6.c. > > > > > > > > This is a regression and I also plan to apply this to gcc-10. > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > PR c++/98355 > > > > * parser.c (cp_parser_has_attribute_expression): Use > > > > value_dependent_expression_p instead of type_dependent_expression_p. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > PR c++/98355 > > > > * g++.dg/ext/builtin-has-attribute2.C: New test. > > > > --- > > > > gcc/cp/parser.c | 2 +- > > > > gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ > > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C > > > > > > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > > > > index 5c1d880c9fc..7b1dc0dc93f 100644 > > > > --- a/gcc/cp/parser.c > > > > +++ b/gcc/cp/parser.c > > > > @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) > > > > { > > > > if (oper == error_mark_node) > > > > /* Nothing. */; > > > > - else if (type_dependent_expression_p (oper)) > > > > + else if (value_dependent_expression_p (oper)) > > > > sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " > > > > "not supported yet"); > > > > > > Actually I don't like this. I think we want > > > processing_template_decl && uses_template_parms () > > > > > > here. > > > > So here's v2. Sorry for the self-review. > > > > -- >8 -- > > __builtin_has_attribute doesn't work in templates yet (bug 92104), so > > in r11-471 I added a sorry. But that only caught type-dependent > > expressions and we also want to sorry on value-dependent expressions. > > This patch uses uses_template_parms, but guarded with p_t_d, because > > u_t_p sets p_t_d and then v_d_e_p considers variables with reference > > types value-dependent, which breaks builtin-has-attribute-6.c. > > Maybe instantiation_dependent_expression_p? That didn't work here because "oper" can be a type, e.g. integer_type, and then potential_constant_expression_1 called from i_d_e_p crashes because it doesn't expect to see a TYPE_P. Marek
On 1/29/21 6:28 PM, Marek Polacek wrote: > On Fri, Jan 29, 2021 at 06:18:35PM -0500, Jason Merrill via Gcc-patches wrote: >> On 1/29/21 5:52 PM, Marek Polacek wrote: >>> On Fri, Jan 29, 2021 at 04:23:14PM -0500, Marek Polacek via Gcc-patches wrote: >>>> On Fri, Jan 29, 2021 at 04:02:51PM -0500, Marek Polacek via Gcc-patches wrote: >>>>> __builtin_has_attribute doesn't work in templates yet (bug 92104), so >>>>> in r11-471 I added a sorry. But that only caught type-dependent >>>>> expressions and we also want to sorry on value-dependent expressions. >>>>> This patch uses v_d_e_p rather than uses_template_parms because u_t_p >>>>> sets p_t_d and then v_d_e_p considers variables with reference types >>>>> value-dependent, which breaks builtin-has-attribute-6.c. >>>>> >>>>> This is a regression and I also plan to apply this to gcc-10. >>>>> >>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> PR c++/98355 >>>>> * parser.c (cp_parser_has_attribute_expression): Use >>>>> value_dependent_expression_p instead of type_dependent_expression_p. >>>>> >>>>> gcc/testsuite/ChangeLog: >>>>> >>>>> PR c++/98355 >>>>> * g++.dg/ext/builtin-has-attribute2.C: New test. >>>>> --- >>>>> gcc/cp/parser.c | 2 +- >>>>> gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ >>>>> 2 files changed, 9 insertions(+), 1 deletion(-) >>>>> create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C >>>>> >>>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c >>>>> index 5c1d880c9fc..7b1dc0dc93f 100644 >>>>> --- a/gcc/cp/parser.c >>>>> +++ b/gcc/cp/parser.c >>>>> @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) >>>>> { >>>>> if (oper == error_mark_node) >>>>> /* Nothing. */; >>>>> - else if (type_dependent_expression_p (oper)) >>>>> + else if (value_dependent_expression_p (oper)) >>>>> sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " >>>>> "not supported yet"); >>>> >>>> Actually I don't like this. I think we want >>>> processing_template_decl && uses_template_parms () >>>> >>>> here. >>> >>> So here's v2. Sorry for the self-review. >>> >>> -- >8 -- >>> __builtin_has_attribute doesn't work in templates yet (bug 92104), so >>> in r11-471 I added a sorry. But that only caught type-dependent >>> expressions and we also want to sorry on value-dependent expressions. >>> This patch uses uses_template_parms, but guarded with p_t_d, because >>> u_t_p sets p_t_d and then v_d_e_p considers variables with reference >>> types value-dependent, which breaks builtin-has-attribute-6.c. >> >> Maybe instantiation_dependent_expression_p? > > That didn't work here because "oper" can be a type, e.g. integer_type, > and then potential_constant_expression_1 called from i_d_e_p crashes > because it doesn't expect to see a TYPE_P. Hmm, if we were calling type_dependent_expression_p with a type argument, that was a bug, and we probably should have aborted; please add such an assert. But then your patch fixes the bug here, so the patch is OK with the assert added. Jason
On Fri, Jan 29, 2021 at 10:31:15PM -0500, Jason Merrill via Gcc-patches wrote: > On 1/29/21 6:28 PM, Marek Polacek wrote: > > On Fri, Jan 29, 2021 at 06:18:35PM -0500, Jason Merrill via Gcc-patches wrote: > > > On 1/29/21 5:52 PM, Marek Polacek wrote: > > > > On Fri, Jan 29, 2021 at 04:23:14PM -0500, Marek Polacek via Gcc-patches wrote: > > > > > On Fri, Jan 29, 2021 at 04:02:51PM -0500, Marek Polacek via Gcc-patches wrote: > > > > > > __builtin_has_attribute doesn't work in templates yet (bug 92104), so > > > > > > in r11-471 I added a sorry. But that only caught type-dependent > > > > > > expressions and we also want to sorry on value-dependent expressions. > > > > > > This patch uses v_d_e_p rather than uses_template_parms because u_t_p > > > > > > sets p_t_d and then v_d_e_p considers variables with reference types > > > > > > value-dependent, which breaks builtin-has-attribute-6.c. > > > > > > > > > > > > This is a regression and I also plan to apply this to gcc-10. > > > > > > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? > > > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > > > PR c++/98355 > > > > > > * parser.c (cp_parser_has_attribute_expression): Use > > > > > > value_dependent_expression_p instead of type_dependent_expression_p. > > > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > > > PR c++/98355 > > > > > > * g++.dg/ext/builtin-has-attribute2.C: New test. > > > > > > --- > > > > > > gcc/cp/parser.c | 2 +- > > > > > > gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ > > > > > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > > > create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C > > > > > > > > > > > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > > > > > > index 5c1d880c9fc..7b1dc0dc93f 100644 > > > > > > --- a/gcc/cp/parser.c > > > > > > +++ b/gcc/cp/parser.c > > > > > > @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) > > > > > > { > > > > > > if (oper == error_mark_node) > > > > > > /* Nothing. */; > > > > > > - else if (type_dependent_expression_p (oper)) > > > > > > + else if (value_dependent_expression_p (oper)) > > > > > > sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " > > > > > > "not supported yet"); > > > > > > > > > > Actually I don't like this. I think we want > > > > > processing_template_decl && uses_template_parms () > > > > > > > > > > here. > > > > > > > > So here's v2. Sorry for the self-review. > > > > > > > > -- >8 -- > > > > __builtin_has_attribute doesn't work in templates yet (bug 92104), so > > > > in r11-471 I added a sorry. But that only caught type-dependent > > > > expressions and we also want to sorry on value-dependent expressions. > > > > This patch uses uses_template_parms, but guarded with p_t_d, because > > > > u_t_p sets p_t_d and then v_d_e_p considers variables with reference > > > > types value-dependent, which breaks builtin-has-attribute-6.c. > > > > > > Maybe instantiation_dependent_expression_p? > > > > That didn't work here because "oper" can be a type, e.g. integer_type, > > and then potential_constant_expression_1 called from i_d_e_p crashes > > because it doesn't expect to see a TYPE_P. > > Hmm, if we were calling type_dependent_expression_p with a type argument, > that was a bug, and we probably should have aborted; please add such an > assert. But then your patch fixes the bug here, so the patch is OK with the > assert added. Thanks. Unfortunately adding gcc_assert (!TYPE_P (expression)); to type_dependent_expression_p causes some amount of ICEs in the testsuite. Can I go ahead with my patch without the assert and deal with the assert later? Marek
On 2/1/21 10:07 AM, Marek Polacek wrote: > On Fri, Jan 29, 2021 at 10:31:15PM -0500, Jason Merrill via Gcc-patches wrote: >> On 1/29/21 6:28 PM, Marek Polacek wrote: >>> On Fri, Jan 29, 2021 at 06:18:35PM -0500, Jason Merrill via Gcc-patches wrote: >>>> On 1/29/21 5:52 PM, Marek Polacek wrote: >>>>> On Fri, Jan 29, 2021 at 04:23:14PM -0500, Marek Polacek via Gcc-patches wrote: >>>>>> On Fri, Jan 29, 2021 at 04:02:51PM -0500, Marek Polacek via Gcc-patches wrote: >>>>>>> __builtin_has_attribute doesn't work in templates yet (bug 92104), so >>>>>>> in r11-471 I added a sorry. But that only caught type-dependent >>>>>>> expressions and we also want to sorry on value-dependent expressions. >>>>>>> This patch uses v_d_e_p rather than uses_template_parms because u_t_p >>>>>>> sets p_t_d and then v_d_e_p considers variables with reference types >>>>>>> value-dependent, which breaks builtin-has-attribute-6.c. >>>>>>> >>>>>>> This is a regression and I also plan to apply this to gcc-10. >>>>>>> >>>>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? >>>>>>> >>>>>>> gcc/cp/ChangeLog: >>>>>>> >>>>>>> PR c++/98355 >>>>>>> * parser.c (cp_parser_has_attribute_expression): Use >>>>>>> value_dependent_expression_p instead of type_dependent_expression_p. >>>>>>> >>>>>>> gcc/testsuite/ChangeLog: >>>>>>> >>>>>>> PR c++/98355 >>>>>>> * g++.dg/ext/builtin-has-attribute2.C: New test. >>>>>>> --- >>>>>>> gcc/cp/parser.c | 2 +- >>>>>>> gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C | 8 ++++++++ >>>>>>> 2 files changed, 9 insertions(+), 1 deletion(-) >>>>>>> create mode 100644 gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C >>>>>>> >>>>>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c >>>>>>> index 5c1d880c9fc..7b1dc0dc93f 100644 >>>>>>> --- a/gcc/cp/parser.c >>>>>>> +++ b/gcc/cp/parser.c >>>>>>> @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) >>>>>>> { >>>>>>> if (oper == error_mark_node) >>>>>>> /* Nothing. */; >>>>>>> - else if (type_dependent_expression_p (oper)) >>>>>>> + else if (value_dependent_expression_p (oper)) >>>>>>> sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " >>>>>>> "not supported yet"); >>>>>> >>>>>> Actually I don't like this. I think we want >>>>>> processing_template_decl && uses_template_parms () >>>>>> >>>>>> here. >>>>> >>>>> So here's v2. Sorry for the self-review. >>>>> >>>>> -- >8 -- >>>>> __builtin_has_attribute doesn't work in templates yet (bug 92104), so >>>>> in r11-471 I added a sorry. But that only caught type-dependent >>>>> expressions and we also want to sorry on value-dependent expressions. >>>>> This patch uses uses_template_parms, but guarded with p_t_d, because >>>>> u_t_p sets p_t_d and then v_d_e_p considers variables with reference >>>>> types value-dependent, which breaks builtin-has-attribute-6.c. >>>> >>>> Maybe instantiation_dependent_expression_p? >>> >>> That didn't work here because "oper" can be a type, e.g. integer_type, >>> and then potential_constant_expression_1 called from i_d_e_p crashes >>> because it doesn't expect to see a TYPE_P. >> >> Hmm, if we were calling type_dependent_expression_p with a type argument, >> that was a bug, and we probably should have aborted; please add such an >> assert. But then your patch fixes the bug here, so the patch is OK with the >> assert added. > > Thanks. Unfortunately adding > > gcc_assert (!TYPE_P (expression)); > > to type_dependent_expression_p causes some amount of ICEs in the testsuite. > > Can I go ahead with my patch without the assert and deal with the assert > later? Yes. Jason
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 5c1d880c9fc..7b1dc0dc93f 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -8934,7 +8934,7 @@ cp_parser_has_attribute_expression (cp_parser *parser) { if (oper == error_mark_node) /* Nothing. */; - else if (type_dependent_expression_p (oper)) + else if (value_dependent_expression_p (oper)) sorry_at (atloc, "%<__builtin_has_attribute%> with dependent argument " "not supported yet"); else diff --git a/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C b/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C new file mode 100644 index 00000000000..aba7932a136 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/builtin-has-attribute2.C @@ -0,0 +1,8 @@ +// PR c++/98355 +// { dg-do compile { target c++11 } } + +struct S { int a; }; +template <int> struct T +{ + static_assert (!__builtin_has_attribute (((S*)0) -> a, packed), ""); // { dg-message "sorry, unimplemented: .__builtin_has_attribute. with dependent argument not supported yet" } +};