tech-toolchain archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: make: sysV subst with variables



On Thu, 2 Jun 2011 17:51:01 +0000, David Holland writes:
> > Sure - these are all from my sys.mk and get used heavily.
> > [snip]
>
>Ok, that makes my eyes burn but I think I understand. 

Which is exactly why you want to capture them in macros,
so that the usage is clear.

>individual modifiers and applied. The bug is that if it sees
>${VAR:${...}} it assumes the internal variable reference is a complete
>modifier (or a complete group of modifiers) and if this isn't true
>because it's e.g. part of a substitution string, it belches.

Yes.

> > So this patch adds a check that what Var_Parse() consumed is what we
> > expect in this case it would not be since '/' is not ':', '}' or ""
> > so it consumes up to one of those, with the result that  ApplyModifiers
> > is called with ${BAR}/%.x=% instead.
>
>Wouldn't it be better to collect the complete modifier text up to the
>terminating right-brace, do a full variable expansion on it, and only

Possibly - ie. always do the above?
The reason I chose to avoid that was just caution.
The existing code has been working well for many years.
My for loop is thus just dealing with a corner case in which we may be
about to fail anyway.

>What happens if you have
>
>   MOD=tl:
>
>(note the trailing colon) and write
>
>   ${FOO:${MOD}S/^/blah/}

This "just works" with or without this patch.
current make gets:

Got 'tl:' from '${MOD}'S/^/bl
...
Result of :S is "blahfoo"

with the patch:

Got 'tl:S/^/blah/' from '${MOD}S/^/blah/'}
...
Result of :S is "blahfoo"

>Worse, what about
>
>   ${FOO:${MOD}S/^/${BAR:tl}/}

Same, this "just works" with or without.
current make:

Got 'tl:' from '${MOD}'S/^/${
...
Result of :S is "barfoo"

with the patch:

Got 'tl:S/^/bar/' from '${MOD}S/^/${BAR:tl}/'}
...
Result of :S is "barfoo"

>Also, currently things like
>
>   FOO=dummy
>   DUM=S/dum
>   ${FOO:${DUM}my/splat/}
>
>are rejected and I think this is a good thing.

This one would work with the patch.

>Given that the svr4 substitution syntax is already a special case,
>maybe the right answer is to make it special in another way and add a
>workaround for cases where one wants the substitution LHS to begin
>with a variable.
>
>Or... hmm, maybe, since we already insist that a variable reference in
>the modifier position is a complete modifier, the right answer is to
>demand that it be followed by : or }, and if it isn't, treat it as
>plain text rather than a modifier block and therefore part of a svr4
>substitution. I believe this is grammatically sound, and it solves
>your problem. It does prohibit the case

Yes, that would work.... not sure how to do it (yet) though.
If it makes thing more complex/ugly is it worth it?
We've not hit any of these corner cases in 5 years.


Home | Main Index | Thread Index | Old Index