On Thu, 21 Mar 2019, Jakub Jelinek wrote: > On Thu, Mar 21, 2019 at 11:13:41AM +0100, Richard Biener wrote: > > > Would you prefer to have say add_tree_to_fld_list that does what it does > > > now > > > and has just 2 callers in find_decls_types_r and a new > > > maybe_add_tree_to_fld_list that would do > > > if (!fld->pset.add (t)) > > > add_tree_to_fld_list (t, fld); > > > and change all the other add_tree_to_fld_list callers? > > > > Not sure if that's needed, if you leave find_decls_types_r alone then > > always doing the check should work, no? > > Well, find_decls_types_r uses add_tree_to_fld_list, so the if (fld->pset.add > (t)) return; can't be in that function. If we want to simplify all the > other callers, so that they don't need to do it manually, then the only > options I see is call some other function in those spots that does this > additional check, or call another function that doesn't do that in the > two spots of find_decls_types_r, or add another argument to the function > say with default argument and test that argument before the fld->pset.add. > E.g. , bool do_add = true and if (do_add && fld->pset.add (t)) return; > and adjust the two calls in find_decls_types_r to call > add_tree_to_fld_list (t, fld, false); > Or keep the patch as is, do the tests before each but the two > add_tree_to_fld_list calls.
I'd have kept the patch but made the add_tree_to_fld_list following the new fld->pset.add (t) calls conditional on the result. Richard.
