Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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 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
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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 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. /// 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], "% 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
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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 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
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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 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.
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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 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
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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 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. /// 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], "% 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], "% 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); }
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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 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
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
Hi, On 26/10/18 17:18, Jason Merrill wrote: On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini 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. / 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], "% can only be specified for variables " @@ -4884,7 +4885,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs, "% 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 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 +struct b { + decltype(a) __attribute__((break)); // { dg-error "declaration does not declare anything" } +};
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini 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
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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.
Re: [C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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
[C++ Patch] PR 84644 ("internal compiler error: in warn_misplaced_attr_for_class_type, at cp/decl.c:4718")
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. /cp 2018-10-15 Paolo Carlini PR c++/84644 * decl.c (check_tag_decl): A decltype with no declarator doesn't declare anything. /testsuite 2018-10-15 Paolo Carlini PR c++/84644 * g++.dg/cpp0x/decltype68.C: New. * g++.dg/cpp0x/decltype-33838.C: Adjust.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 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 +struct b { + decltype(a) __attribute__((break)); // { dg-error "declaration does not declare anything" } +};