On Sun, 2020-05-24 at 11:22 -0500, Segher Boessenkool wrote: > Hi! > > On Sun, May 24, 2020 at 07:03:13PM +0530, Kamlesh Kumar wrote: > > In this patch series trying to address same by creating a struct Tuple > > which bundles existing rtx and machine_mode and added one more > > bool member which store unsigned_p which by default is false. > > The idea is good. However, you cannot call something as specific as this > "tuple", in a header file that is used everywhere even. (We also do not > have a "leading caps on types" convention). > > > This patch does not change underlying behavior yet. This will be done in > > follow up patches. > > Thanks :-) > > > * rtl.h (Tuple): Defined and typedefed to rtx_mode_t. > > It's the other way around: rtx_mode_t is typedeffed to struct Tuple, so > rtx_mode_t should be listed to the left of a : as well. > > OTOH, you don't need to name Tuple at all... It should not *have* a > constructor, since you declared it as class... But you can just use > std::tuple here? > > > (emit_library_call): Added default arg unsigned_p. > > (emit_library_call_value): Added default arg unsigned_p. > > Yeah, eww. Default arguments have all the problems you had before, > except now it is hidden and much more surprising. > > Those functions really should take rtx_mode_t arguments? > > Thanks again for working on this, ISTM that using std::tuple would be better than defining our own types.
I'd rather see the argument be explicit rather than using default arguments too. While I have ack'd some patches with default arguments, I still don't like 'em. I do like the approach of getting the infrastructure in place without changing behavior, then having the behavior fix as a distinct change. jeff
