| Message ID | 660300ac-0dfb-5104-2606-cface7b37d70@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Consistently use OVL_P | expand |
On Tue, May 14, 2019 at 05:28:09PM +0200, Paolo Carlini wrote: > Hi, > > another straightforward one sitting in my tree... Sanity checked on > x86_64-linux. > > Thanks, Paolo. > > /////////////////// > > 2019-05-14 Paolo Carlini <paolo.carlini@oracle.com> > > * call.c (perform_overload_resolution, build_new_method_call_1): > Use OVL_P. > * decl.c (grokfndecl): Likewise. > * mangle.c (write_expression): Likewise. > * parser.c (cp_parser_template_id): Likewise. > * pt.c (resolve_overloaded_unification, type_dependent_expression_p): > Likewise. > * search.c (build_baselink): Likewise. > * tree.c (is_overloaded_fn, dependent_name, maybe_get_fns): Likewise. Looks fine. OVL_P says "TEMPLATE_DECLS are always wrapped in an OVERLOAD, so we don't need to check them", but I wouldn't mess with that in a cleanup patch like this. Marek
On 5/14/19 11:28 AM, Paolo Carlini wrote: > another straightforward one sitting in my tree... Sanity checked on > x86_64-linux. I suspect many/all of the TREE_CODE (x) == TEMPLATE_DECL (or DECL_FUNCTION_TEMPLATE_P) could also be elided -- we don't have naked function templates at that point, they're always wrapped in overloads. Could you see if that's true? nathan
On Tue, May 14, 2019 at 03:01:35PM -0400, Nathan Sidwell wrote: > On 5/14/19 11:28 AM, Paolo Carlini wrote: > > > another straightforward one sitting in my tree... Sanity checked on > > x86_64-linux. > > I suspect many/all of the TREE_CODE (x) == TEMPLATE_DECL > (or DECL_FUNCTION_TEMPLATE_P) could also be elided -- we don't have naked > function templates at that point, they're always wrapped in overloads. > Could you see if that's true? That's what I pointed out here <https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00718.html> but I thought it might be better to do it as a follow-up. Marek
Hi, On 14/05/19 21:05, Marek Polacek wrote: > On Tue, May 14, 2019 at 03:01:35PM -0400, Nathan Sidwell wrote: >> On 5/14/19 11:28 AM, Paolo Carlini wrote: >> >>> another straightforward one sitting in my tree... Sanity checked on >>> x86_64-linux. >> I suspect many/all of the TREE_CODE (x) == TEMPLATE_DECL >> (or DECL_FUNCTION_TEMPLATE_P) could also be elided -- we don't have naked >> function templates at that point, they're always wrapped in overloads. >> Could you see if that's true? > That's what I pointed out here > <https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00718.html> > but I thought it might be better to do it as a follow-up. Yeah, on the other hand, we can as well sort out this now: after all, I like to spend time on this kind of work also in the hope that little buglets or issues will then become more evident. Thus, I'm removing all those checks for TEMPLATE_DECL and DECL_FUNCTION_TEMPLATE_P, let's see what regression testing tells us. To be clear: I'm leaving TEMPLATE_ID_EXPR (two of those) alone. Paolo.
Hi again, On 14/05/19 21:21, Paolo Carlini wrote: > Hi, > > On 14/05/19 21:05, Marek Polacek wrote: >> On Tue, May 14, 2019 at 03:01:35PM -0400, Nathan Sidwell wrote: >>> On 5/14/19 11:28 AM, Paolo Carlini wrote: >>> >>>> another straightforward one sitting in my tree... Sanity checked on >>>> x86_64-linux. >>> I suspect many/all of the TREE_CODE (x) == TEMPLATE_DECL >>> (or DECL_FUNCTION_TEMPLATE_P) could also be elided -- we don't have >>> naked >>> function templates at that point, they're always wrapped in overloads. >>> Could you see if that's true? >> That's what I pointed out here >> <https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00718.html> >> but I thought it might be better to do it as a follow-up. > > Yeah, on the other hand, we can as well sort out this now: after all, > I like to spend time on this kind of work also in the hope that little > buglets or issues will then become more evident. Thus, I'm removing > all those checks for TEMPLATE_DECL and DECL_FUNCTION_TEMPLATE_P, let's > see what regression testing tells us. To be clear: I'm leaving > TEMPLATE_ID_EXPR (two of those) alone. ... so the below passes testing on x86_64-linux. In fact, I think we are on a pretty safe ground, now at the beginning of Stage 1: if, over the next months we get a testcase which causes one of the 4 tightened gcc_assert to trip we'll comfortably deal with it. Thanks, Paolo. //////////////////// Index: call.c =================================================================== --- call.c (revision 271166) +++ call.c (working copy) @@ -4383,9 +4383,7 @@ perform_overload_resolution (tree fn, *any_viable_p = true; /* Check FN. */ - gcc_assert (TREE_CODE (fn) == FUNCTION_DECL - || TREE_CODE (fn) == TEMPLATE_DECL - || TREE_CODE (fn) == OVERLOAD + gcc_assert (OVL_P (fn) || TREE_CODE (fn) == TEMPLATE_ID_EXPR); if (TREE_CODE (fn) == TEMPLATE_ID_EXPR) @@ -9605,9 +9603,7 @@ build_new_method_call_1 (tree instance, tree fns, fns = TREE_OPERAND (fns, 0); template_only = 1; } - gcc_assert (TREE_CODE (fns) == FUNCTION_DECL - || TREE_CODE (fns) == TEMPLATE_DECL - || TREE_CODE (fns) == OVERLOAD); + gcc_assert (OVL_P (fns)); fn = OVL_FIRST (fns); name = DECL_NAME (fn); Index: decl.c =================================================================== --- decl.c (revision 271166) +++ decl.c (working copy) @@ -8918,9 +8918,7 @@ grokfndecl (tree ctype, the information in the TEMPLATE_ID_EXPR. */ SET_DECL_IMPLICIT_INSTANTIATION (decl); - gcc_assert (identifier_p (fns) - || TREE_CODE (fns) == OVERLOAD - || TREE_CODE (fns) == FUNCTION_DECL); + gcc_assert (identifier_p (fns) || OVL_P (fns)); DECL_TEMPLATE_INFO (decl) = build_template_info (fns, args); for (t = TYPE_ARG_TYPES (TREE_TYPE (decl)); t; t = TREE_CHAIN (t)) Index: mangle.c =================================================================== --- mangle.c (revision 271166) +++ mangle.c (working copy) @@ -3278,8 +3278,7 @@ write_expression (tree expr) /* Mangle a dependent name as the name, not whatever happens to be the first function in the overload set. */ - if ((TREE_CODE (fn) == FUNCTION_DECL - || TREE_CODE (fn) == OVERLOAD) + if (OVL_P (fn) && type_dependent_expression_p_push (expr)) fn = OVL_NAME (fn); Index: parser.c =================================================================== --- parser.c (revision 271166) +++ parser.c (working copy) @@ -16479,10 +16479,8 @@ cp_parser_template_id (cp_parser *parser, { /* If it's not a class-template or a template-template, it should be a function-template. */ - gcc_assert ((DECL_FUNCTION_TEMPLATE_P (templ) - || TREE_CODE (templ) == OVERLOAD - || TREE_CODE (templ) == FUNCTION_DECL - || BASELINK_P (templ))); + gcc_assert (OVL_P (templ) + || BASELINK_P (templ)); template_id = lookup_template_function (templ, arguments); if (TREE_CODE (template_id) == TEMPLATE_ID_EXPR) Index: pt.c =================================================================== --- pt.c (revision 271166) +++ pt.c (working copy) @@ -21193,8 +21193,7 @@ resolve_overloaded_unification (tree tparms, if (good != 1) good = ok; } - else if (TREE_CODE (arg) != OVERLOAD - && TREE_CODE (arg) != FUNCTION_DECL) + else if (!OVL_P (arg)) /* If ARG is, for example, "(0, &f)" then its type will be unknown -- but the deduction does not succeed because the expression is not just the function on its own. */ @@ -25950,8 +25949,7 @@ type_dependent_expression_p (tree expression) return true; } - gcc_assert (TREE_CODE (expression) == OVERLOAD - || TREE_CODE (expression) == FUNCTION_DECL); + gcc_assert (OVL_P (expression)); for (lkp_iterator iter (expression); iter; ++iter) if (type_dependent_expression_p (*iter)) Index: search.c =================================================================== --- search.c (revision 271166) +++ search.c (working copy) @@ -1058,10 +1058,8 @@ build_baselink (tree binfo, tree access_binfo, tre { tree baselink; - gcc_assert (TREE_CODE (functions) == FUNCTION_DECL - || TREE_CODE (functions) == TEMPLATE_DECL - || TREE_CODE (functions) == TEMPLATE_ID_EXPR - || TREE_CODE (functions) == OVERLOAD); + gcc_assert (OVL_P (functions) + || TREE_CODE (functions) == TEMPLATE_ID_EXPR); gcc_assert (!optype || TYPE_P (optype)); gcc_assert (TREE_TYPE (functions)); Index: tree.c =================================================================== --- tree.c (revision 271166) +++ tree.c (working copy) @@ -2381,8 +2381,7 @@ is_overloaded_fn (tree x) || (TREE_CODE (x) == OVERLOAD && !OVL_SINGLE_P (x))) return 2; - return (TREE_CODE (x) == FUNCTION_DECL - || TREE_CODE (x) == OVERLOAD); + return OVL_P (x); } /* X is the CALL_EXPR_FN of a CALL_EXPR. If X represents a dependent name @@ -2396,7 +2395,7 @@ dependent_name (tree x) return x; if (TREE_CODE (x) == TEMPLATE_ID_EXPR) x = TREE_OPERAND (x, 0); - if (TREE_CODE (x) == OVERLOAD || TREE_CODE (x) == FUNCTION_DECL) + if (OVL_P (x)) return OVL_NAME (x); return NULL_TREE; } @@ -2428,8 +2427,7 @@ maybe_get_fns (tree from) if (TREE_CODE (from) == TEMPLATE_ID_EXPR) from = TREE_OPERAND (from, 0); - if (TREE_CODE (from) == OVERLOAD - || TREE_CODE (from) == FUNCTION_DECL) + if (OVL_P (from)) return from; return NULL;
On Tue, May 14, 2019 at 11:20:16PM +0200, Paolo Carlini wrote: > Hi again, > > On 14/05/19 21:21, Paolo Carlini wrote: > > Hi, > > > > On 14/05/19 21:05, Marek Polacek wrote: > > > On Tue, May 14, 2019 at 03:01:35PM -0400, Nathan Sidwell wrote: > > > > On 5/14/19 11:28 AM, Paolo Carlini wrote: > > > > > > > > > another straightforward one sitting in my tree... Sanity checked on > > > > > x86_64-linux. > > > > I suspect many/all of the TREE_CODE (x) == TEMPLATE_DECL > > > > (or DECL_FUNCTION_TEMPLATE_P) could also be elided -- we don't > > > > have naked > > > > function templates at that point, they're always wrapped in overloads. > > > > Could you see if that's true? > > > That's what I pointed out here > > > <https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00718.html> > > > but I thought it might be better to do it as a follow-up. > > > > Yeah, on the other hand, we can as well sort out this now: after all, I > > like to spend time on this kind of work also in the hope that little > > buglets or issues will then become more evident. Thus, I'm removing all > > those checks for TEMPLATE_DECL and DECL_FUNCTION_TEMPLATE_P, let's see > > what regression testing tells us. To be clear: I'm leaving > > TEMPLATE_ID_EXPR (two of those) alone. > > ... so the below passes testing on x86_64-linux. In fact, I think we are on > a pretty safe ground, now at the beginning of Stage 1: if, over the next > months we get a testcase which causes one of the 4 tightened gcc_assert to > trip we'll comfortably deal with it. Nice! Just nits: > Index: call.c > =================================================================== > --- call.c (revision 271166) > +++ call.c (working copy) > @@ -4383,9 +4383,7 @@ perform_overload_resolution (tree fn, > *any_viable_p = true; > > /* Check FN. */ > - gcc_assert (TREE_CODE (fn) == FUNCTION_DECL > - || TREE_CODE (fn) == TEMPLATE_DECL > - || TREE_CODE (fn) == OVERLOAD > + gcc_assert (OVL_P (fn) > || TREE_CODE (fn) == TEMPLATE_ID_EXPR); This can now fit on one line. > Index: parser.c > =================================================================== > --- parser.c (revision 271166) > +++ parser.c (working copy) > @@ -16479,10 +16479,8 @@ cp_parser_template_id (cp_parser *parser, > { > /* If it's not a class-template or a template-template, it should be > a function-template. */ > - gcc_assert ((DECL_FUNCTION_TEMPLATE_P (templ) > - || TREE_CODE (templ) == OVERLOAD > - || TREE_CODE (templ) == FUNCTION_DECL > - || BASELINK_P (templ))); > + gcc_assert (OVL_P (templ) > + || BASELINK_P (templ)); Likewise. > Index: search.c > =================================================================== > --- search.c (revision 271166) > +++ search.c (working copy) > @@ -1058,10 +1058,8 @@ build_baselink (tree binfo, tree access_binfo, tre > { > tree baselink; > > - gcc_assert (TREE_CODE (functions) == FUNCTION_DECL > - || TREE_CODE (functions) == TEMPLATE_DECL > - || TREE_CODE (functions) == TEMPLATE_ID_EXPR > - || TREE_CODE (functions) == OVERLOAD); > + gcc_assert (OVL_P (functions) > + || TREE_CODE (functions) == TEMPLATE_ID_EXPR); Likewise. Marek
On 5/14/19 5:20 PM, Paolo Carlini wrote: > Hi again, > > ... so the below passes testing on x86_64-linux. In fact, I think we are > on a pretty safe ground, now at the beginning of Stage 1: if, over the > next months we get a testcase which causes one of the 4 tightened > gcc_assert to trip we'll comfortably deal with it. great, thanks for trying that. Marek found the nits. nathan
Hi, On 15/05/19 14:29, Nathan Sidwell wrote: > On 5/14/19 5:20 PM, Paolo Carlini wrote: >> Hi again, >> >> ... so the below passes testing on x86_64-linux. In fact, I think we >> are on a pretty safe ground, now at the beginning of Stage 1: if, >> over the next months we get a testcase which causes one of the 4 >> tightened gcc_assert to trip we'll comfortably deal with it. > > great, thanks for trying that. Marek found the nits. Thus, I'm going ahead with those obvious formatting changes, agreed? Paolo.
On Wed, May 15, 2019 at 03:24:52PM +0200, Paolo Carlini wrote: > Hi, > > On 15/05/19 14:29, Nathan Sidwell wrote: > > On 5/14/19 5:20 PM, Paolo Carlini wrote: > > > Hi again, > > > > > > ... so the below passes testing on x86_64-linux. In fact, I think we > > > are on a pretty safe ground, now at the beginning of Stage 1: if, > > > over the next months we get a testcase which causes one of the 4 > > > tightened gcc_assert to trip we'll comfortably deal with it. > > > > great, thanks for trying that. Marek found the nits. > > Thus, I'm going ahead with those obvious formatting changes, agreed? Go ahead. Marek
Index: call.c =================================================================== --- call.c (revision 271166) +++ call.c (working copy) @@ -4383,9 +4383,8 @@ perform_overload_resolution (tree fn, *any_viable_p = true; /* Check FN. */ - gcc_assert (TREE_CODE (fn) == FUNCTION_DECL + gcc_assert (OVL_P (fn) || TREE_CODE (fn) == TEMPLATE_DECL - || TREE_CODE (fn) == OVERLOAD || TREE_CODE (fn) == TEMPLATE_ID_EXPR); if (TREE_CODE (fn) == TEMPLATE_ID_EXPR) @@ -9605,9 +9604,8 @@ build_new_method_call_1 (tree instance, tree fns, fns = TREE_OPERAND (fns, 0); template_only = 1; } - gcc_assert (TREE_CODE (fns) == FUNCTION_DECL - || TREE_CODE (fns) == TEMPLATE_DECL - || TREE_CODE (fns) == OVERLOAD); + gcc_assert (OVL_P (fns) + || TREE_CODE (fns) == TEMPLATE_DECL); fn = OVL_FIRST (fns); name = DECL_NAME (fn); Index: decl.c =================================================================== --- decl.c (revision 271166) +++ decl.c (working copy) @@ -8918,9 +8918,7 @@ grokfndecl (tree ctype, the information in the TEMPLATE_ID_EXPR. */ SET_DECL_IMPLICIT_INSTANTIATION (decl); - gcc_assert (identifier_p (fns) - || TREE_CODE (fns) == OVERLOAD - || TREE_CODE (fns) == FUNCTION_DECL); + gcc_assert (identifier_p (fns) || OVL_P (fns)); DECL_TEMPLATE_INFO (decl) = build_template_info (fns, args); for (t = TYPE_ARG_TYPES (TREE_TYPE (decl)); t; t = TREE_CHAIN (t)) Index: mangle.c =================================================================== --- mangle.c (revision 271166) +++ mangle.c (working copy) @@ -3278,8 +3278,7 @@ write_expression (tree expr) /* Mangle a dependent name as the name, not whatever happens to be the first function in the overload set. */ - if ((TREE_CODE (fn) == FUNCTION_DECL - || TREE_CODE (fn) == OVERLOAD) + if (OVL_P (fn) && type_dependent_expression_p_push (expr)) fn = OVL_NAME (fn); Index: parser.c =================================================================== --- parser.c (revision 271166) +++ parser.c (working copy) @@ -16479,10 +16479,9 @@ cp_parser_template_id (cp_parser *parser, { /* If it's not a class-template or a template-template, it should be a function-template. */ - gcc_assert ((DECL_FUNCTION_TEMPLATE_P (templ) - || TREE_CODE (templ) == OVERLOAD - || TREE_CODE (templ) == FUNCTION_DECL - || BASELINK_P (templ))); + gcc_assert (DECL_FUNCTION_TEMPLATE_P (templ) + || OVL_P (templ) + || BASELINK_P (templ)); template_id = lookup_template_function (templ, arguments); if (TREE_CODE (template_id) == TEMPLATE_ID_EXPR) Index: pt.c =================================================================== --- pt.c (revision 271166) +++ pt.c (working copy) @@ -21193,8 +21193,7 @@ resolve_overloaded_unification (tree tparms, if (good != 1) good = ok; } - else if (TREE_CODE (arg) != OVERLOAD - && TREE_CODE (arg) != FUNCTION_DECL) + else if (!OVL_P (arg)) /* If ARG is, for example, "(0, &f)" then its type will be unknown -- but the deduction does not succeed because the expression is not just the function on its own. */ @@ -25950,8 +25949,7 @@ type_dependent_expression_p (tree expression) return true; } - gcc_assert (TREE_CODE (expression) == OVERLOAD - || TREE_CODE (expression) == FUNCTION_DECL); + gcc_assert (OVL_P (expression)); for (lkp_iterator iter (expression); iter; ++iter) if (type_dependent_expression_p (*iter)) Index: search.c =================================================================== --- search.c (revision 271166) +++ search.c (working copy) @@ -1058,10 +1058,9 @@ build_baselink (tree binfo, tree access_binfo, tre { tree baselink; - gcc_assert (TREE_CODE (functions) == FUNCTION_DECL + gcc_assert (OVL_P (functions) || TREE_CODE (functions) == TEMPLATE_DECL - || TREE_CODE (functions) == TEMPLATE_ID_EXPR - || TREE_CODE (functions) == OVERLOAD); + || TREE_CODE (functions) == TEMPLATE_ID_EXPR); gcc_assert (!optype || TYPE_P (optype)); gcc_assert (TREE_TYPE (functions)); Index: tree.c =================================================================== --- tree.c (revision 271166) +++ tree.c (working copy) @@ -2381,8 +2381,7 @@ is_overloaded_fn (tree x) || (TREE_CODE (x) == OVERLOAD && !OVL_SINGLE_P (x))) return 2; - return (TREE_CODE (x) == FUNCTION_DECL - || TREE_CODE (x) == OVERLOAD); + return OVL_P (x); } /* X is the CALL_EXPR_FN of a CALL_EXPR. If X represents a dependent name @@ -2396,7 +2395,7 @@ dependent_name (tree x) return x; if (TREE_CODE (x) == TEMPLATE_ID_EXPR) x = TREE_OPERAND (x, 0); - if (TREE_CODE (x) == OVERLOAD || TREE_CODE (x) == FUNCTION_DECL) + if (OVL_P (x)) return OVL_NAME (x); return NULL_TREE; } @@ -2428,8 +2427,7 @@ maybe_get_fns (tree from) if (TREE_CODE (from) == TEMPLATE_ID_EXPR) from = TREE_OPERAND (from, 0); - if (TREE_CODE (from) == OVERLOAD - || TREE_CODE (from) == FUNCTION_DECL) + if (OVL_P (from)) return from; return NULL;