This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH: More Makefile cleanups
Mark Mitchell wrote:
On Fri, 2003-08-29 at 19:57, DJ Delorie wrote:
+ $(genprogs:%=%$(buildextext)): \
I believe this is the type of textual obfuscation that we were worried
about ;-)
I'm slightly confused by your and Nathanael's comments on this patch.
Why is this considered bad?
That idea allowed me to avoid writing the same list of gen* in two
separate places.
Do you mean that you didn't like having this on the target side of the
rule?
In my case, that's part of it. I think that this type of textual
substitution [$(x:%=%), patsubst, etc.], although useful, has truly
cryptic notation, and each usage should therefore be commented
specifically with its intended purpose.
When embedded into a rule, it's *very* hard to debug what it actually
evaluates to. When isolated, it's not so bad, as long as it's commented.
(Nathanael's revision broke the manipulation out into a separate
variable, which I don't see as clearly better, but I'm perfectly content
with either form.)
Breaking it out separately makes it easier to debug, since the value of
the macro can be checked. Embedded in a rule in this way, it's not at
all easy to check that it's doing the right thing.
I do understand Nathanael's desire for comments, although I will note
that this code was totally uncommented before
I admit that I'm being a bit aggressive about comments. Most of the
build system is very poorly commented, so whenever a significant change
is made, I like to at least have basic comments on the purpose of the
code segments.
-- and a couple of the
proposed comments are incorrect. (These are not *all* of the programs
that run on the build system, so this comment:
# Dependencies shared by all the programs built to run on the
# build system
is false. There are a couple that have different dependencies;
especially those that depend on BUILD_EARLY_SUPPORT.)
How interesting. I had no idea. :-)
I don't understand why Nathanael suggested separating the dependencies
for these programs like so:
$(genprogs): $(BUILD_RTL) $(BUILD_SUPPORT) $(BUILD_PRINT) \
$(BUILD_ERRORS) $(BUILD_LIBDEPS)
out of the static pattern rule.
Why is it better to have multiple sets of dependencies? By including
them in the static pattern rule, there are fewer places to look to see
what the dependencies are.
This is probably an arguable point. I'm not picky as long as the
indentation makes a clear separation between dependencies and commands.
Like so, for example:
$(genprogs): %$(build_exeext): %.o $(BUILD_RTL) $(BUILD_SUPPORT) \
$(BUILD_PRINT) $(BUILD_ERRORS) \
$(BUILD_LIBDEPS)
$(CC_FOR_BUILD) $(BUILD_CFLAGS) $(BUILD_LDFLAGS) -o $@ $< \
$(BUILD_RTL) $(BUILD_SUPPORT) $(BUILD_PRINT) $(BUILD_ERRORS) \
$(BUILD_LIBS)
There are probably other good indentation styles, but this is the one
I've generally found readable, and it corresponds conceptually to the
GNU Coding Standards recommendation for functions whose formal
parameters take more than one line.
It also contains an important typo.
Nathanael, what typo? I couldn't see it looking at your proposed fix,
so I imagine I'm going blind.
+ $(genprogs:%=%$(buildextext)):
^^^^^^^^^^^
should be build_exeext, of course. Being buried inside an already
complicated rule makes it hard to spot, though, doesn't it?
I'm perfectly happy with Nathanael's version; I just want to understand
what the criteria being used are.
In my case, clarity for the future, even to someone used to BSD Make.
Both Nathanael's version and mind
remove immense amounts of duplicated code, which is *my* primary
criteria.
(I'd rather not be trying to iterate a lot on these patches; this is
purely volunteerism on my part, and my time for such projects is very
scarce; if I have to do iterations, I may just skip it.