make-alpha
[Top][All Lists]
Advanced

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

Re: About the last patch for private variables


From: Ramón García
Subject: Re: About the last patch for private variables
Date: Tue, 26 May 2009 19:58:23 +0200

Thanks.

I am having exams until 15th June. Nevertheless, I will try to look at it.

2009/5/26 Paul Smith <address@hidden>:
> On Thu, 2009-04-09 at 18:00 +0200, Ramón García wrote:
>> I have sent a patch, which I believe to be complete, for the
>> implementation of private variables. I have not received any news
>> about whether it was committed or not.
>
> Hi Ramon.  Over the long weekend I found some time to work on this, and
> was able to get it committed.  Please try it out and make sure I didn't
> break anything: I had to make a few changes as the patch did more than
> just implement this (there were extra changes like removing the page
> feeds (^L) etc.)
>
> Also, the code to allow multiple modifiers (the while loop) caused some
> problems, and using a modifier like "private" means that variable and
> prerequisite names "private" didn't work properly which is not good
> (backward-compatibility problems).  I noticed we had the same problem
> previously with other modifiers like "export" and "override", and also
> some limitations (you couldn't use both "export" and "override" on the
> same variable assignment).  Also there was some code duplication in the
> parser.
>
> As a result it seem like time to fix the variable assignment parser to
> resolve these issues, so that's what took the extra time.  You can now
> say things like:
>
>        export override foo = bar
>
>        export = foo
>
>        private override export foo = bar
>
>        override export define foo
>        endef
>
> etc.  Note this also extends the capability of "private" to apply to
> globally-scoped variables as well.  What does this mean?  It means that
> global variables marked private are available in other global contexts
> but NOT available inside a target context (when expanding a recipe).  I
> don't know if this is very useful but it's consistent which is good.
>
> The changes required two small backward-compatibility issues: first,
> using backslash to escape "=" in prerequisite lists doesn't work
> anymore.  That was a bad decision on my part, since that syntax never
> worked for normal variables.  If you want to create a target (or
> variable) with an "=" in the name you have to hide it behind a variable:
>        E = =
>        foo: bar$(E)baz
>
> That always worked.  Second, you can no longer create a variable name
> with whitespace in it just by declaring it like this:
>        foo bar = baz
> This is now a syntax error.  You can do it with a variable, if you must,
> although in the future we may disallow variable names containing
> whitespace altogether.
>
> Also everyone please test this and make sure I didn't break anything.
> Also I think that the new algorithm is not significantly slower than the
> previous one but it's probably worth testing on any massive makefiles
> you happen to have lying around (esp. on makefiles with lots of
> variables assignments).
>
> This _almost_ allows for target-specific define/endef variables.  In
> order to make this work there are a few things that need to be fixed:
> first, the do_define() and record_target_var() functions need to be
> brought into sync so the former can be called in the context of the
> latter (or whatever).  This is not hard.  But the big problem is the
> second one: handling ignored target-specific defines in a conditional.
>
> We have to know when we're in a define even when we're ignoring content,
> because otherwise things like this break:
>
>        ifeq (1,0)
>          define foo
>            ifeq (1,1)
>              $(info here)
>            endif
>          endef
>        endif
>
> If we don't parse enough of the ignored content of the if-condition to
> realize that we're inside a define/endef, then the parser chooses the
> FIRST endif as the end of the condition.  There is code currently to
> handle this in the "normal" variable assignment parser; even in ignored
> sections we parse enough to know if it's a define and if so we set a
> flag in_ignored_define so we ignore EVERYTHING until we see an endef.
>
> We would need to handle this somehow for target-specific defines to
> work; however, this is much harder since today if we're ignoring we skip
> to the next line long before we realize this is a target that might have
> a target-specific variable attached to it.
>
> I think the answer here is two-fold: first we should change the regular
> behavior so that rather than keeping in_ignored_define, we ALWAYS invoke
> do_define() and just pass it an argument to say whether we're ignoring
> or not.
>
> Second we have to modify the way we skip lines in a false conditional so
> we can recognize a target-specific define and handle it properly.
>
> I'm not suggesting anyone needs to work on this right now; just thinking
> out loud...
>
> --
> -------------------------------------------------------------------------------
>  Paul D. Smith <address@hidden>          Find some GNU make tips at:
>  http://www.gnu.org                      http://make.mad-scientist.us
>  "Please remain calm...I may be mad, but I am a professional." --Mad Scientist
>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]