This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Reformatting and minor code changes for dwarf2out.c


>>>>> "Richard" == Richard Kenner <kenner@vlsi1.ultra.nyu.edu> writes:

>     Speaking as the dwarf2out maintainer, I agree with Richard.

> Umm... we're both "Richard".

Er, so you are.  I suppose I'm used to thinking of you as "Kenner".  :)

>     I very seldom think that abstraction makes the code harder to read.

> It doesn't if it's carried through.  Here we had exactly *one*
> abstract operation, used exactly once.  That was more "abstraction for
> the sake of abstraction".  Moreover, that one usage is *explicitly*
> deadling with incomplete types, unlike when the purpose of the code is
> to do something else and you want to abstract away some sub-operation.

I don't think an abstraction is necessarily invalid because it's only used
once.  It is conceivable that we might want to change how these types are
recorded at some point--indeed, I'm pretty sure it used to use a linked
list--and with it in a separate function, the main logic doesn't need to
change.  Abstract data types and all that.  But this is a general point; I
don't really care much about this instance.

>     The same goes for save_rtl, which you removed in a previous patch.

> That one I realized afterwards would probably have been better to just
> change to take the rtx as a parameter and return void.  With GC, it
> didn't make an sense any more as a function returning rtl since a
> caller tends to want to know sometimes whether an rtl has been
> modified or not.  This one is a case where indeed the abstraction of a
> sub-operation makes more sense.  But I don't think it's worth putting
> back at this point unless you do.

I suppose it isn't important.  But don't remove any other abstractions,
please.

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]