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:

>     Please do not un-factor code like this.  I find the later much
>     harder to read.  In addition, with ENABLE_RTL_CHECKING, this
>     runs slower.

> My reasoning is that it's a tradeoff between code being simpler and being
> smaller.  Both simpler and smaller make the code easier to read.  Here they
> are in conflict and I made the judgement that smaller was preferred here,
> though, as you've pointed out, one could easily make the opposite
> judgement.

Speaking as the dwarf2out maintainer, I agree with Richard.

>     - static void
>     - add_incomplete_type (type)
>     -      tree type;
>     - {
>     -   VARRAY_PUSH_TREE (incomplete_types, type);
>     - }

>     Why?  Yes, it was used one place, so what?  Mark it inline if you must.

> It was used in one place *and* was one line long.  It wasn't an efficiency
> issue, but one where I felt the additional level of abstraction made the
> code harder to read not easier.

I very seldom think that abstraction makes the code harder to read.
Usually it doesn't matter how incomplete types are recorded in order to
follow the logic of the caller.  Not that it's a particularly important
abstraction in this case, but I don't see any reason to remove it either.
The same goes for save_rtl, which you removed in a previous patch.

Jason


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