* [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") @ 2018-10-15 18:21 Paolo Carlini 2018-10-24 21:56 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Paolo Carlini @ 2018-10-15 18:21 UTC (permalink / raw) To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 347 bytes --] Hi, here we ICE when, at the end of check_tag_decl we pass a DECLTYPE_TYPE to warn_misplaced_attr_for_class_type. I think the right fix is rejecting earlier a decltype with no declarator as a declaration that doesn't declare anything (note: all the compilers I have at hand agree). Tested x86_64-linux. Thanks, Paolo. //////////////////// [-- Attachment #2: CL_84644 --] [-- Type: text/plain, Size: 315 bytes --] /cp 2018-10-15 Paolo Carlini <paolo.carlini@oracle.com> PR c++/84644 * decl.c (check_tag_decl): A decltype with no declarator doesn't declare anything. /testsuite 2018-10-15 Paolo Carlini <paolo.carlini@oracle.com> PR c++/84644 * g++.dg/cpp0x/decltype68.C: New. * g++.dg/cpp0x/decltype-33838.C: Adjust. [-- Attachment #3: patch_84644 --] [-- Type: text/plain, Size: 1405 bytes --] Index: cp/decl.c =================================================================== --- cp/decl.c (revision 265158) +++ cp/decl.c (working copy) @@ -4793,6 +4793,7 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, if (declspecs->type && TYPE_P (declspecs->type) && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE + && TREE_CODE (declspecs->type) != DECLTYPE_TYPE && MAYBE_CLASS_TYPE_P (declspecs->type)) || TREE_CODE (declspecs->type) == ENUMERAL_TYPE)) declared_type = declspecs->type; Index: testsuite/g++.dg/cpp0x/decltype-33838.C =================================================================== --- testsuite/g++.dg/cpp0x/decltype-33838.C (revision 265158) +++ testsuite/g++.dg/cpp0x/decltype-33838.C (working copy) @@ -2,5 +2,5 @@ // PR c++/33838 template<typename T> struct A { - __decltype (T* foo()); // { dg-error "expected|no arguments|accept" } + __decltype (T* foo()); // { dg-error "expected|no arguments|declaration" } }; Index: testsuite/g++.dg/cpp0x/decltype68.C =================================================================== --- testsuite/g++.dg/cpp0x/decltype68.C (nonexistent) +++ testsuite/g++.dg/cpp0x/decltype68.C (working copy) @@ -0,0 +1,7 @@ +// PR c++/84644 +// { dg-do compile { target c++11 } } + +template<int a> +struct b { + decltype(a) __attribute__((break)); // { dg-error "declaration does not declare anything" } +}; ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-10-15 18:21 [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") Paolo Carlini @ 2018-10-24 21:56 ` Jason Merrill 2018-10-26 9:37 ` Paolo Carlini 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2018-10-24 21:56 UTC (permalink / raw) To: Paolo Carlini, gcc-patches; +Cc: Nathan Sidwell On 10/15/18 12:45 PM, Paolo Carlini wrote: > && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE > + && TREE_CODE (declspecs->type) != DECLTYPE_TYPE > && MAYBE_CLASS_TYPE_P (declspecs->type)) I would think that the MAYBE_CLASS_TYPE_P here should be CLASS_TYPE_P, and then we can remove the TYPENAME_TYPE check. Or do we want to allow template type parameters for some reason? Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-10-24 21:56 ` Jason Merrill @ 2018-10-26 9:37 ` Paolo Carlini 2018-10-26 16:28 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Paolo Carlini @ 2018-10-26 9:37 UTC (permalink / raw) To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell Hi, On 24/10/18 22:41, Jason Merrill wrote: > On 10/15/18 12:45 PM, Paolo Carlini wrote: >>        && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >> +      && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>         && MAYBE_CLASS_TYPE_P (declspecs->type)) > > I would think that the MAYBE_CLASS_TYPE_P here should be CLASS_TYPE_P, > and then we can remove the TYPENAME_TYPE check. Or do we want to > allow template type parameters for some reason? Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it seems we at least want to let through TEMPLATE_TYPE_PARMs representing 'auto' - otherwise Dodji's check a few lines below which fixed c++/51473 doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, otherwise we regress on template/spec32.C and template/ttp22.C because we don't diagnose the shadowing anymore. Thus, I would say either we keep on using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add a comment? Thanks, Paolo. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-10-26 9:37 ` Paolo Carlini @ 2018-10-26 16:28 ` Jason Merrill 2018-10-26 19:03 ` Paolo Carlini 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2018-10-26 16:28 UTC (permalink / raw) To: Paolo Carlini; +Cc: gcc-patches List, Nathan Sidwell On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini <paolo.carlini@oracle.com> wrote: > On 24/10/18 22:41, Jason Merrill wrote: > > On 10/15/18 12:45 PM, Paolo Carlini wrote: > >> && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE > >> + && TREE_CODE (declspecs->type) != DECLTYPE_TYPE > >> && MAYBE_CLASS_TYPE_P (declspecs->type)) > > > > I would think that the MAYBE_CLASS_TYPE_P here should be CLASS_TYPE_P, > > and then we can remove the TYPENAME_TYPE check. Or do we want to > > allow template type parameters for some reason? > > Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it seems > we at least want to let through TEMPLATE_TYPE_PARMs representing 'auto' > - otherwise Dodji's check a few lines below which fixed c++/51473 > doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, otherwise > we regress on template/spec32.C and template/ttp22.C because we don't > diagnose the shadowing anymore. Thus, I would say either we keep on > using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add a comment? Aha. I guess the answer is not to restrict that test any more, but instead to fix the code further down so it gives a proper diagnostic rather than call warn_misplaced_attr_for_class_type. Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-10-26 16:28 ` Jason Merrill @ 2018-10-26 19:03 ` Paolo Carlini 2018-10-30 21:46 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Paolo Carlini @ 2018-10-26 19:03 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 1414 bytes --] Hi, On 26/10/18 17:18, Jason Merrill wrote: > On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini <paolo.carlini@oracle.com> wrote: >> On 24/10/18 22:41, Jason Merrill wrote: >>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>> && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>> + && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>> && MAYBE_CLASS_TYPE_P (declspecs->type)) >>> I would think that the MAYBE_CLASS_TYPE_P here should be CLASS_TYPE_P, >>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>> allow template type parameters for some reason? >> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it seems >> we at least want to let through TEMPLATE_TYPE_PARMs representing 'auto' >> - otherwise Dodji's check a few lines below which fixed c++/51473 >> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, otherwise >> we regress on template/spec32.C and template/ttp22.C because we don't >> diagnose the shadowing anymore. Thus, I would say either we keep on >> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add a comment? > Aha. I guess the answer is not to restrict that test any more, but > instead to fix the code further down so it gives a proper diagnostic > rather than call warn_misplaced_attr_for_class_type. I see. Thus something like the below? It passes testing on x86_64-linux. Thanks! Paolo. ///////////// [-- Attachment #2: patch_84644_2 --] [-- Type: text/plain, Size: 2093 bytes --] Index: cp/decl.c =================================================================== --- cp/decl.c (revision 265510) +++ cp/decl.c (working copy) @@ -4798,9 +4798,10 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, declared_type = declspecs->type; else if (declspecs->type == error_mark_node) error_p = true; - if (declared_type == NULL_TREE && ! saw_friend && !error_p) + if ((!declared_type || TREE_CODE (declared_type) == DECLTYPE_TYPE) + && ! saw_friend && !error_p) permerror (input_location, "declaration does not declare anything"); - else if (declared_type != NULL_TREE && type_uses_auto (declared_type)) + else if (declared_type && type_uses_auto (declared_type)) { error_at (declspecs->locations[ds_type_spec], "%<auto%> can only be specified for variables " @@ -4884,7 +4885,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, "%<constexpr%> cannot be used for type declarations"); } - if (declspecs->attributes && warn_attributes && declared_type) + if (declspecs->attributes && warn_attributes && declared_type + && TREE_CODE (declared_type) != DECLTYPE_TYPE) { location_t loc; if (!CLASS_TYPE_P (declared_type) Index: testsuite/g++.dg/cpp0x/decltype-33838.C =================================================================== --- testsuite/g++.dg/cpp0x/decltype-33838.C (revision 265510) +++ testsuite/g++.dg/cpp0x/decltype-33838.C (working copy) @@ -2,5 +2,5 @@ // PR c++/33838 template<typename T> struct A { - __decltype (T* foo()); // { dg-error "expected|no arguments|accept" } + __decltype (T* foo()); // { dg-error "expected|no arguments|declaration" } }; Index: testsuite/g++.dg/cpp0x/decltype68.C =================================================================== --- testsuite/g++.dg/cpp0x/decltype68.C (nonexistent) +++ testsuite/g++.dg/cpp0x/decltype68.C (working copy) @@ -0,0 +1,7 @@ +// PR c++/84644 +// { dg-do compile { target c++11 } } + +template<int a> +struct b { + decltype(a) __attribute__((break)); // { dg-error "declaration does not declare anything" } +}; ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-10-26 19:03 ` Paolo Carlini @ 2018-10-30 21:46 ` Jason Merrill 2018-10-31 5:25 ` Paolo Carlini 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2018-10-30 21:46 UTC (permalink / raw) To: Paolo Carlini; +Cc: gcc-patches List, Nathan Sidwell On 10/26/18 2:02 PM, Paolo Carlini wrote: > On 26/10/18 17:18, Jason Merrill wrote: >> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini >> <paolo.carlini@oracle.com> wrote: >>> On 24/10/18 22:41, Jason Merrill wrote: >>>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>>>         && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>>> +      && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>>>          && MAYBE_CLASS_TYPE_P (declspecs->type)) >>>> I would think that the MAYBE_CLASS_TYPE_P here should be CLASS_TYPE_P, >>>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>>> allow template type parameters for some reason? >>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it seems >>> we at least want to let through TEMPLATE_TYPE_PARMs representing 'auto' >>> - otherwise Dodji's check a few lines below which fixed c++/51473 >>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, otherwise >>> we regress on template/spec32.C and template/ttp22.C because we don't >>> diagnose the shadowing anymore. Thus, I would say either we keep on >>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add a >>> comment? >> Aha. I guess the answer is not to restrict that test any more, but >> instead to fix the code further down so it gives a proper diagnostic >> rather than call warn_misplaced_attr_for_class_type. > > I see. Thus something like the below? It passes testing on x86_64-linux. > + if ((!declared_type || TREE_CODE (declared_type) == DECLTYPE_TYPE) > + && ! saw_friend && !error_p) > permerror (input_location, "declaration does not declare anything"); I see no reason to make this specific to decltype. Maybe move this diagnostic into the final 'else' block with the other declspec diagnostics and not look at declared_type at all? > + if (declspecs->attributes && warn_attributes && declared_type > + && TREE_CODE (declared_type) != DECLTYPE_TYPE) I think we do want to give a diagnostic about useless attributes, not skip it. Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-10-30 21:46 ` Jason Merrill @ 2018-10-31 5:25 ` Paolo Carlini 2018-12-13 22:22 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Paolo Carlini @ 2018-10-31 5:25 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 2871 bytes --] Hi, On 30/10/18 21:37, Jason Merrill wrote: > On 10/26/18 2:02 PM, Paolo Carlini wrote: >> On 26/10/18 17:18, Jason Merrill wrote: >>> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini >>> <paolo.carlini@oracle.com> wrote: >>>> On 24/10/18 22:41, Jason Merrill wrote: >>>>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>>>>         && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>>>> +      && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>>>>          && MAYBE_CLASS_TYPE_P (declspecs->type)) >>>>> I would think that the MAYBE_CLASS_TYPE_P here should be >>>>> CLASS_TYPE_P, >>>>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>>>> allow template type parameters for some reason? >>>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it seems >>>> we at least want to let through TEMPLATE_TYPE_PARMs representing >>>> 'auto' >>>> - otherwise Dodji's check a few lines below which fixed c++/51473 >>>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, >>>> otherwise >>>> we regress on template/spec32.C and template/ttp22.C because we don't >>>> diagnose the shadowing anymore. Thus, I would say either we keep on >>>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add a >>>> comment? >>> Aha. I guess the answer is not to restrict that test any more, but >>> instead to fix the code further down so it gives a proper diagnostic >>> rather than call warn_misplaced_attr_for_class_type. >> >> I see. Thus something like the below? It passes testing on x86_64-linux. > >> + if ((!declared_type || TREE_CODE (declared_type) == DECLTYPE_TYPE) >> +     && ! saw_friend && !error_p) >>     permerror (input_location, "declaration does not declare >> anything"); > > I see no reason to make this specific to decltype. Maybe move this > diagnostic into the final 'else' block with the other declspec > diagnostics and not look at declared_type at all? I'm not sure to fully understand: if we do that we still want to at least minimally check that declared_type is null, like we already do, and then we simply accept the new testcase. Is that Ok? Because, as I probably mentioned at some point, all the other compilers I have at hand issue a "does not declare anything" diagnostic, and we likewise do that for the legacy __typeof. Not looking into declared_type *at all* doesn't work with plain class types and enums, of course. Or you meant something entirely different?? >> + if (declspecs->attributes && warn_attributes && declared_type >> +     && TREE_CODE (declared_type) != DECLTYPE_TYPE) > > I think we do want to give a diagnostic about useless attributes, not > skip it. Agreed. FWIW the attached tests fine. Thanks, Paolo. /////////////////// [-- Attachment #2: ppp --] [-- Type: text/plain, Size: 1495 bytes --] Index: decl.c =================================================================== --- decl.c (revision 265636) +++ decl.c (working copy) @@ -4798,9 +4798,7 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, declared_type = declspecs->type; else if (declspecs->type == error_mark_node) error_p = true; - if (declared_type == NULL_TREE && ! saw_friend && !error_p) - permerror (input_location, "declaration does not declare anything"); - else if (declared_type != NULL_TREE && type_uses_auto (declared_type)) + if (declared_type && type_uses_auto (declared_type)) { error_at (declspecs->locations[ds_type_spec], "%<auto%> can only be specified for variables " @@ -4842,7 +4840,9 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, else { - if (decl_spec_seq_has_spec_p (declspecs, ds_inline)) + if (!declared_type && ! saw_friend && !error_p) + permerror (input_location, "declaration does not declare anything"); + else if (decl_spec_seq_has_spec_p (declspecs, ds_inline)) error_at (declspecs->locations[ds_inline], "%<inline%> can only be specified for functions"); else if (decl_spec_seq_has_spec_p (declspecs, ds_virtual)) @@ -4909,7 +4909,7 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, "no attribute can be applied to " "an explicit instantiation"); } - else + else if (TREE_CODE (declared_type) != DECLTYPE_TYPE) warn_misplaced_attr_for_class_type (loc, declared_type); } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-10-31 5:25 ` Paolo Carlini @ 2018-12-13 22:22 ` Jason Merrill 2018-12-14 18:44 ` Paolo Carlini 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2018-12-13 22:22 UTC (permalink / raw) To: Paolo Carlini; +Cc: gcc-patches List, Nathan Sidwell On 10/30/18 9:22 PM, Paolo Carlini wrote: > Hi, > > On 30/10/18 21:37, Jason Merrill wrote: >> On 10/26/18 2:02 PM, Paolo Carlini wrote: >>> On 26/10/18 17:18, Jason Merrill wrote: >>>> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini >>>> <paolo.carlini@oracle.com> wrote: >>>>> On 24/10/18 22:41, Jason Merrill wrote: >>>>>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>>>>>         && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>>>>> +      && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>>>>>          && MAYBE_CLASS_TYPE_P (declspecs->type)) >>>>>> I would think that the MAYBE_CLASS_TYPE_P here should be >>>>>> CLASS_TYPE_P, >>>>>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>>>>> allow template type parameters for some reason? >>>>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it seems >>>>> we at least want to let through TEMPLATE_TYPE_PARMs representing >>>>> 'auto' >>>>> - otherwise Dodji's check a few lines below which fixed c++/51473 >>>>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, >>>>> otherwise >>>>> we regress on template/spec32.C and template/ttp22.C because we don't >>>>> diagnose the shadowing anymore. Thus, I would say either we keep on >>>>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add a >>>>> comment? >>>> Aha. I guess the answer is not to restrict that test any more, but >>>> instead to fix the code further down so it gives a proper diagnostic >>>> rather than call warn_misplaced_attr_for_class_type. >>> >>> I see. Thus something like the below? It passes testing on x86_64-linux. >> >>> + if ((!declared_type || TREE_CODE (declared_type) == DECLTYPE_TYPE) >>> +     && ! saw_friend && !error_p) >>>     permerror (input_location, "declaration does not declare >>> anything"); >> >> I see no reason to make this specific to decltype. Maybe move this >> diagnostic into the final 'else' block with the other declspec >> diagnostics and not look at declared_type at all? > > I'm not sure to fully understand: if we do that we still want to at > least minimally check that declared_type is null, like we already do, > and then we simply accept the new testcase. Is that Ok? Because, as I > probably mentioned at some point, all the other compilers I have at hand > issue a "does not declare anything" diagnostic, and we likewise do that > for the legacy __typeof. Not looking into declared_type *at all* doesn't > work with plain class types and enums, of course. Or you meant something > entirely different?? > >>> + if (declspecs->attributes && warn_attributes && declared_type >>> +     && TREE_CODE (declared_type) != DECLTYPE_TYPE) >> >> I think we do want to give a diagnostic about useless attributes, not >> skip it. > > Agreed. FWIW the attached tests fine. The problem here is that the code toward the bottom expects "declared_type" to be the tagged type declared by a declaration with no declarator, and in this testcase it's ending up as a DECLTYPE_TYPE. I think once we've checked for 'auto' we don't want declared_type to be anything that isn't OVERLOAD_TYPE_P. We can arrange that either by checking for 'auto' first and then changing the code that sets declared_type to use OVERLOAD_TYPE_P, or by clearing declared_type after checking for 'auto' if it isn't OVERLOAD_TYPE_P. Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-12-13 22:22 ` Jason Merrill @ 2018-12-14 18:44 ` Paolo Carlini 2018-12-14 20:19 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Paolo Carlini @ 2018-12-14 18:44 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Nathan Sidwell Hi, On 13/12/18 22:03, Jason Merrill wrote: > On 10/30/18 9:22 PM, Paolo Carlini wrote: >> Hi, >> >> On 30/10/18 21:37, Jason Merrill wrote: >>> On 10/26/18 2:02 PM, Paolo Carlini wrote: >>>> On 26/10/18 17:18, Jason Merrill wrote: >>>>> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini >>>>> <paolo.carlini@oracle.com> wrote: >>>>>> On 24/10/18 22:41, Jason Merrill wrote: >>>>>>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>>>>>>         && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>>>>>> +      && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>>>>>>          && MAYBE_CLASS_TYPE_P (declspecs->type)) >>>>>>> I would think that the MAYBE_CLASS_TYPE_P here should be >>>>>>> CLASS_TYPE_P, >>>>>>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>>>>>> allow template type parameters for some reason? >>>>>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it >>>>>> seems >>>>>> we at least want to let through TEMPLATE_TYPE_PARMs representing >>>>>> 'auto' >>>>>> - otherwise Dodji's check a few lines below which fixed c++/51473 >>>>>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, >>>>>> otherwise >>>>>> we regress on template/spec32.C and template/ttp22.C because we >>>>>> don't >>>>>> diagnose the shadowing anymore. Thus, I would say either we keep on >>>>>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add >>>>>> a comment? >>>>> Aha. I guess the answer is not to restrict that test any more, but >>>>> instead to fix the code further down so it gives a proper diagnostic >>>>> rather than call warn_misplaced_attr_for_class_type. >>>> >>>> I see. Thus something like the below? It passes testing on >>>> x86_64-linux. >>> >>>> + if ((!declared_type || TREE_CODE (declared_type) == DECLTYPE_TYPE) >>>> +     && ! saw_friend && !error_p) >>>>     permerror (input_location, "declaration does not declare >>>> anything"); >>> >>> I see no reason to make this specific to decltype. Maybe move this >>> diagnostic into the final 'else' block with the other declspec >>> diagnostics and not look at declared_type at all? >> >> I'm not sure to fully understand: if we do that we still want to at >> least minimally check that declared_type is null, like we already do, >> and then we simply accept the new testcase. Is that Ok? Because, as I >> probably mentioned at some point, all the other compilers I have at >> hand issue a "does not declare anything" diagnostic, and we likewise >> do that for the legacy __typeof. Not looking into declared_type *at >> all* doesn't work with plain class types and enums, of course. Or you >> meant something entirely different?? >> >>>> + if (declspecs->attributes && warn_attributes && declared_type >>>> +     && TREE_CODE (declared_type) != DECLTYPE_TYPE) >>> >>> I think we do want to give a diagnostic about useless attributes, >>> not skip it. >> >> Agreed. FWIW the attached tests fine. > > The problem here is that the code toward the bottom expects > "declared_type" to be the tagged type declared by a declaration with > no declarator, and in this testcase it's ending up as a DECLTYPE_TYPE. > > I think once we've checked for 'auto' we don't want declared_type to > be anything that isn't OVERLOAD_TYPE_P. We can arrange that either by > checking for 'auto' first and then changing the code that sets > declared_type to use OVERLOAD_TYPE_P, or by clearing declared_type > after checking for 'auto' if it isn't OVERLOAD_TYPE_P. Thanks. I'm slowly catching up on this issue... Any suggestion about BOUND_TEMPLATE_TEMPLATE_PARM? If we don't let through such tree nodes - which are MAYBE_CLASS_TYPE_P and aren't OVERLOAD_TYPE_P - we regress on template/spec32.C, we don't reject it anymore., Paolo. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-12-14 18:44 ` Paolo Carlini @ 2018-12-14 20:19 ` Jason Merrill 2018-12-14 21:34 ` Paolo Carlini 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2018-12-14 20:19 UTC (permalink / raw) To: Paolo Carlini; +Cc: gcc-patches List, Nathan Sidwell On 12/14/18 1:44 PM, Paolo Carlini wrote: > Hi, > > On 13/12/18 22:03, Jason Merrill wrote: >> On 10/30/18 9:22 PM, Paolo Carlini wrote: >>> Hi, >>> >>> On 30/10/18 21:37, Jason Merrill wrote: >>>> On 10/26/18 2:02 PM, Paolo Carlini wrote: >>>>> On 26/10/18 17:18, Jason Merrill wrote: >>>>>> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini >>>>>> <paolo.carlini@oracle.com> wrote: >>>>>>> On 24/10/18 22:41, Jason Merrill wrote: >>>>>>>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>>>>>>>         && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>>>>>>> +      && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>>>>>>>          && MAYBE_CLASS_TYPE_P (declspecs->type)) >>>>>>>> I would think that the MAYBE_CLASS_TYPE_P here should be >>>>>>>> CLASS_TYPE_P, >>>>>>>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>>>>>>> allow template type parameters for some reason? >>>>>>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it >>>>>>> seems >>>>>>> we at least want to let through TEMPLATE_TYPE_PARMs representing >>>>>>> 'auto' >>>>>>> - otherwise Dodji's check a few lines below which fixed c++/51473 >>>>>>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, >>>>>>> otherwise >>>>>>> we regress on template/spec32.C and template/ttp22.C because we >>>>>>> don't >>>>>>> diagnose the shadowing anymore. Thus, I would say either we keep on >>>>>>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add >>>>>>> a comment? >>>>>> Aha. I guess the answer is not to restrict that test any more, but >>>>>> instead to fix the code further down so it gives a proper diagnostic >>>>>> rather than call warn_misplaced_attr_for_class_type. >>>>> >>>>> I see. Thus something like the below? It passes testing on >>>>> x86_64-linux. >>>> >>>>> + if ((!declared_type || TREE_CODE (declared_type) == DECLTYPE_TYPE) >>>>> +     && ! saw_friend && !error_p) >>>>>     permerror (input_location, "declaration does not declare >>>>> anything"); >>>> >>>> I see no reason to make this specific to decltype. Maybe move this >>>> diagnostic into the final 'else' block with the other declspec >>>> diagnostics and not look at declared_type at all? >>> >>> I'm not sure to fully understand: if we do that we still want to at >>> least minimally check that declared_type is null, like we already do, >>> and then we simply accept the new testcase. Is that Ok? Because, as I >>> probably mentioned at some point, all the other compilers I have at >>> hand issue a "does not declare anything" diagnostic, and we likewise >>> do that for the legacy __typeof. Not looking into declared_type *at >>> all* doesn't work with plain class types and enums, of course. Or you >>> meant something entirely different?? >>> >>>>> + if (declspecs->attributes && warn_attributes && declared_type >>>>> +     && TREE_CODE (declared_type) != DECLTYPE_TYPE) >>>> >>>> I think we do want to give a diagnostic about useless attributes, >>>> not skip it. >>> >>> Agreed. FWIW the attached tests fine. >> >> The problem here is that the code toward the bottom expects >> "declared_type" to be the tagged type declared by a declaration with >> no declarator, and in this testcase it's ending up as a DECLTYPE_TYPE. >> >> I think once we've checked for 'auto' we don't want declared_type to >> be anything that isn't OVERLOAD_TYPE_P. We can arrange that either by >> checking for 'auto' first and then changing the code that sets >> declared_type to use OVERLOAD_TYPE_P, or by clearing declared_type >> after checking for 'auto' if it isn't OVERLOAD_TYPE_P. > > Thanks. I'm slowly catching up on this issue... Any suggestion about > BOUND_TEMPLATE_TEMPLATE_PARM? If we don't let through such tree nodes - > which are MAYBE_CLASS_TYPE_P and aren't OVERLOAD_TYPE_P - we regress on > template/spec32.C, we don't reject it anymore. If we clear declared_type for a BOUND_TEMPLATE_TEMPLATE_PARM, we should get the "does not declare anything" error. Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-12-14 20:19 ` Jason Merrill @ 2018-12-14 21:34 ` Paolo Carlini 2018-12-14 21:43 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Paolo Carlini @ 2018-12-14 21:34 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 4488 bytes --] Hi, On 14/12/18 21:19, Jason Merrill wrote: > On 12/14/18 1:44 PM, Paolo Carlini wrote: >> Hi, >> >> On 13/12/18 22:03, Jason Merrill wrote: >>> On 10/30/18 9:22 PM, Paolo Carlini wrote: >>>> Hi, >>>> >>>> On 30/10/18 21:37, Jason Merrill wrote: >>>>> On 10/26/18 2:02 PM, Paolo Carlini wrote: >>>>>> On 26/10/18 17:18, Jason Merrill wrote: >>>>>>> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini >>>>>>> <paolo.carlini@oracle.com> wrote: >>>>>>>> On 24/10/18 22:41, Jason Merrill wrote: >>>>>>>>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>>>>>>>>         && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>>>>>>>> +      && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>>>>>>>>          && MAYBE_CLASS_TYPE_P (declspecs->type)) >>>>>>>>> I would think that the MAYBE_CLASS_TYPE_P here should be >>>>>>>>> CLASS_TYPE_P, >>>>>>>>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>>>>>>>> allow template type parameters for some reason? >>>>>>>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However >>>>>>>> it seems >>>>>>>> we at least want to let through TEMPLATE_TYPE_PARMs >>>>>>>> representing 'auto' >>>>>>>> - otherwise Dodji's check a few lines below which fixed c++/51473 >>>>>>>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, >>>>>>>> otherwise >>>>>>>> we regress on template/spec32.C and template/ttp22.C because we >>>>>>>> don't >>>>>>>> diagnose the shadowing anymore. Thus, I would say either we >>>>>>>> keep on >>>>>>>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we >>>>>>>> add a comment? >>>>>>> Aha. I guess the answer is not to restrict that test any more, but >>>>>>> instead to fix the code further down so it gives a proper >>>>>>> diagnostic >>>>>>> rather than call warn_misplaced_attr_for_class_type. >>>>>> >>>>>> I see. Thus something like the below? It passes testing on >>>>>> x86_64-linux. >>>>> >>>>>> + if ((!declared_type || TREE_CODE (declared_type) == >>>>>> DECLTYPE_TYPE) >>>>>> +     && ! saw_friend && !error_p) >>>>>>     permerror (input_location, "declaration does not declare >>>>>> anything"); >>>>> >>>>> I see no reason to make this specific to decltype. Maybe move >>>>> this diagnostic into the final 'else' block with the other >>>>> declspec diagnostics and not look at declared_type at all? >>>> >>>> I'm not sure to fully understand: if we do that we still want to at >>>> least minimally check that declared_type is null, like we already >>>> do, and then we simply accept the new testcase. Is that Ok? >>>> Because, as I probably mentioned at some point, all the other >>>> compilers I have at hand issue a "does not declare anything" >>>> diagnostic, and we likewise do that for the legacy __typeof. Not >>>> looking into declared_type *at all* doesn't work with plain class >>>> types and enums, of course. Or you meant something entirely >>>> different?? >>>> >>>>>> + if (declspecs->attributes && warn_attributes && declared_type >>>>>> +     && TREE_CODE (declared_type) != DECLTYPE_TYPE) >>>>> >>>>> I think we do want to give a diagnostic about useless attributes, >>>>> not skip it. >>>> >>>> Agreed. FWIW the attached tests fine. >>> >>> The problem here is that the code toward the bottom expects >>> "declared_type" to be the tagged type declared by a declaration with >>> no declarator, and in this testcase it's ending up as a DECLTYPE_TYPE. >>> >>> I think once we've checked for 'auto' we don't want declared_type to >>> be anything that isn't OVERLOAD_TYPE_P. We can arrange that either >>> by checking for 'auto' first and then changing the code that sets >>> declared_type to use OVERLOAD_TYPE_P, or by clearing declared_type >>> after checking for 'auto' if it isn't OVERLOAD_TYPE_P. >> >> Thanks. I'm slowly catching up on this issue... Any suggestion about >> BOUND_TEMPLATE_TEMPLATE_PARM? If we don't let through such tree nodes >> - which are MAYBE_CLASS_TYPE_P and aren't OVERLOAD_TYPE_P - we >> regress on template/spec32.C, we don't reject it anymore. > If we clear declared_type for a BOUND_TEMPLATE_TEMPLATE_PARM, we > should get the "does not declare anything" error. Ah, now I see, I didn't realize that we would also change the errors we issue for those testcases. Thus the below is finishing testing, appears to work fine. Thanks, Paolo. /////////////////// [-- Attachment #2: patch_84644_3 --] [-- Type: text/plain, Size: 3075 bytes --] Index: cp/decl.c =================================================================== --- cp/decl.c (revision 267131) +++ cp/decl.c (working copy) @@ -4803,9 +4803,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, declared_type = declspecs->type; else if (declspecs->type == error_mark_node) error_p = true; - if (declared_type == NULL_TREE && ! saw_friend && !error_p) - permerror (input_location, "declaration does not declare anything"); - else if (declared_type != NULL_TREE && type_uses_auto (declared_type)) + + if (type_uses_auto (declared_type)) { error_at (declspecs->locations[ds_type_spec], "%<auto%> can only be specified for variables " @@ -4812,6 +4811,12 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, "or function declarations"); return error_mark_node; } + + if (declared_type && !OVERLOAD_TYPE_P (declared_type)) + declared_type = NULL_TREE; + + if (!declared_type && !saw_friend && !error_p) + permerror (input_location, "declaration does not declare anything"); /* Check for an anonymous union. */ else if (declared_type && RECORD_OR_UNION_CODE_P (TREE_CODE (declared_type)) && TYPE_UNNAMED_P (declared_type)) Index: testsuite/g++.dg/cpp0x/decltype-33838.C =================================================================== --- testsuite/g++.dg/cpp0x/decltype-33838.C (revision 267127) +++ testsuite/g++.dg/cpp0x/decltype-33838.C (working copy) @@ -2,5 +2,5 @@ // PR c++/33838 template<typename T> struct A { - __decltype (T* foo()); // { dg-error "expected|no arguments|accept" } + __decltype (T* foo()); // { dg-error "expected|no arguments|declaration" } }; Index: testsuite/g++.dg/cpp0x/decltype68.C =================================================================== --- testsuite/g++.dg/cpp0x/decltype68.C (nonexistent) +++ testsuite/g++.dg/cpp0x/decltype68.C (working copy) @@ -0,0 +1,7 @@ +// PR c++/84644 +// { dg-do compile { target c++11 } } + +template<int a> +struct b { + decltype(a) __attribute__((break)); // { dg-error "declaration does not declare anything" } +}; Index: testsuite/g++.dg/template/spec32.C =================================================================== --- testsuite/g++.dg/template/spec32.C (revision 267127) +++ testsuite/g++.dg/template/spec32.C (working copy) @@ -2,5 +2,5 @@ struct A { - template<template<int> class B> struct B<0>; // { dg-error "name of class shadows" } + template<template<int> class B> struct B<0>; // { dg-error "declaration does not declare anything" } }; Index: testsuite/g++.dg/template/ttp22.C =================================================================== --- testsuite/g++.dg/template/ttp22.C (revision 267127) +++ testsuite/g++.dg/template/ttp22.C (working copy) @@ -2,7 +2,7 @@ // { dg-do compile } template<template<int> class A> -class A<0>; // { dg-error "shadows template template parameter" } +class A<0>; // { dg-error "declaration does not declare anything" } template<template<int> class B> class B<0> {}; // { dg-error "shadows template template parameter" } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") 2018-12-14 21:34 ` Paolo Carlini @ 2018-12-14 21:43 ` Jason Merrill 0 siblings, 0 replies; 12+ messages in thread From: Jason Merrill @ 2018-12-14 21:43 UTC (permalink / raw) To: Paolo Carlini; +Cc: gcc-patches List, Nathan Sidwell On 12/14/18 4:33 PM, Paolo Carlini wrote: > Hi, > > On 14/12/18 21:19, Jason Merrill wrote: >> On 12/14/18 1:44 PM, Paolo Carlini wrote: >>> Hi, >>> >>> On 13/12/18 22:03, Jason Merrill wrote: >>>> On 10/30/18 9:22 PM, Paolo Carlini wrote: >>>>> Hi, >>>>> >>>>> On 30/10/18 21:37, Jason Merrill wrote: >>>>>> On 10/26/18 2:02 PM, Paolo Carlini wrote: >>>>>>> On 26/10/18 17:18, Jason Merrill wrote: >>>>>>>> On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini >>>>>>>> <paolo.carlini@oracle.com> wrote: >>>>>>>>> On 24/10/18 22:41, Jason Merrill wrote: >>>>>>>>>> On 10/15/18 12:45 PM, Paolo Carlini wrote: >>>>>>>>>>>         && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE >>>>>>>>>>> +      && TREE_CODE (declspecs->type) != DECLTYPE_TYPE >>>>>>>>>>>          && MAYBE_CLASS_TYPE_P (declspecs->type)) >>>>>>>>>> I would think that the MAYBE_CLASS_TYPE_P here should be >>>>>>>>>> CLASS_TYPE_P, >>>>>>>>>> and then we can remove the TYPENAME_TYPE check. Or do we want to >>>>>>>>>> allow template type parameters for some reason? >>>>>>>>> Indeed, it would be nice to just use OVERLOAD_TYPE_P. However >>>>>>>>> it seems >>>>>>>>> we at least want to let through TEMPLATE_TYPE_PARMs >>>>>>>>> representing 'auto' >>>>>>>>> - otherwise Dodji's check a few lines below which fixed c++/51473 >>>>>>>>> doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, >>>>>>>>> otherwise >>>>>>>>> we regress on template/spec32.C and template/ttp22.C because we >>>>>>>>> don't >>>>>>>>> diagnose the shadowing anymore. Thus, I would say either we >>>>>>>>> keep on >>>>>>>>> using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we >>>>>>>>> add a comment? >>>>>>>> Aha. I guess the answer is not to restrict that test any more, but >>>>>>>> instead to fix the code further down so it gives a proper >>>>>>>> diagnostic >>>>>>>> rather than call warn_misplaced_attr_for_class_type. >>>>>>> >>>>>>> I see. Thus something like the below? It passes testing on >>>>>>> x86_64-linux. >>>>>> >>>>>>> + if ((!declared_type || TREE_CODE (declared_type) == >>>>>>> DECLTYPE_TYPE) >>>>>>> +     && ! saw_friend && !error_p) >>>>>>>     permerror (input_location, "declaration does not declare >>>>>>> anything"); >>>>>> >>>>>> I see no reason to make this specific to decltype. Maybe move >>>>>> this diagnostic into the final 'else' block with the other >>>>>> declspec diagnostics and not look at declared_type at all? >>>>> >>>>> I'm not sure to fully understand: if we do that we still want to at >>>>> least minimally check that declared_type is null, like we already >>>>> do, and then we simply accept the new testcase. Is that Ok? >>>>> Because, as I probably mentioned at some point, all the other >>>>> compilers I have at hand issue a "does not declare anything" >>>>> diagnostic, and we likewise do that for the legacy __typeof. Not >>>>> looking into declared_type *at all* doesn't work with plain class >>>>> types and enums, of course. Or you meant something entirely >>>>> different?? >>>>> >>>>>>> + if (declspecs->attributes && warn_attributes && declared_type >>>>>>> +     && TREE_CODE (declared_type) != DECLTYPE_TYPE) >>>>>> >>>>>> I think we do want to give a diagnostic about useless attributes, >>>>>> not skip it. >>>>> >>>>> Agreed. FWIW the attached tests fine. >>>> >>>> The problem here is that the code toward the bottom expects >>>> "declared_type" to be the tagged type declared by a declaration with >>>> no declarator, and in this testcase it's ending up as a DECLTYPE_TYPE. >>>> >>>> I think once we've checked for 'auto' we don't want declared_type to >>>> be anything that isn't OVERLOAD_TYPE_P. We can arrange that either >>>> by checking for 'auto' first and then changing the code that sets >>>> declared_type to use OVERLOAD_TYPE_P, or by clearing declared_type >>>> after checking for 'auto' if it isn't OVERLOAD_TYPE_P. >>> >>> Thanks. I'm slowly catching up on this issue... Any suggestion about >>> BOUND_TEMPLATE_TEMPLATE_PARM? If we don't let through such tree nodes >>> - which are MAYBE_CLASS_TYPE_P and aren't OVERLOAD_TYPE_P - we >>> regress on template/spec32.C, we don't reject it anymore. >> If we clear declared_type for a BOUND_TEMPLATE_TEMPLATE_PARM, we >> should get the "does not declare anything" error. > > Ah, now I see, I didn't realize that we would also change the errors we > issue for those testcases. Thus the below is finishing testing, appears > to work fine. OK. Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-12-14 21:43 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-15 18:21 [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718") Paolo Carlini 2018-10-24 21:56 ` Jason Merrill 2018-10-26 9:37 ` Paolo Carlini 2018-10-26 16:28 ` Jason Merrill 2018-10-26 19:03 ` Paolo Carlini 2018-10-30 21:46 ` Jason Merrill 2018-10-31 5:25 ` Paolo Carlini 2018-12-13 22:22 ` Jason Merrill 2018-12-14 18:44 ` Paolo Carlini 2018-12-14 20:19 ` Jason Merrill 2018-12-14 21:34 ` Paolo Carlini 2018-12-14 21:43 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).