[Top][All Lists]

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

Re: Patch: Fix for ltdeps.test

From: Akim Demaille
Subject: Re: Patch: Fix for ltdeps.test
Date: 03 Jun 2001 15:56:42 +0200
User-agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.1 (GTK)

| Akim, what do you think of this patch?

Eeeck!  The condition_* subs!  That's a very obscure area, and I fear
I forgot most about it :(

| Take a look at ltdeps.test.  For this test, we were getting `TRUE' as
| the only condition for BuildSources, which is clearly wrong.  


| Part of the patch is just moving code out of variable_conditions_sub.
| That part is correct.

Yep, that's much easier to read as you propose.

| I also think the changes in variable_conditions_sub are correct.
| However I'm less sure of them.

Same here :( Someday, we should probably completely rewrite this.
That's one of my utopias once we spread the use of objects :) And
since we also want to rewrite the handling of conditionals... (to
avoid the combinatorial explosion for a start).

| I've noticed that if I remove the second loop in variable_conditions_sub
| (the one "If we are being called on behalf of another variable..."),
| then the three tests I was working with (ltdeps, cond3, and cond4) all
| still pass.  I wonder if this code is required.  I don't understand it
| any more :-(.  I chose not to remove it (yet).  The test suite passes
| with the appended patch.

Be careful, I seem to remember I had the same thought, and ISTR I
reintroduced it.  I don't remember why.  Maybe cvs annotate knows
[checking...] no, there is nothing related to me there.  Or maybe was
it related to:

2001-04-09  Akim Demaille  <address@hidden>

        * (&handle_source_transform):  Use
        &variable_conditions, don't read $conditional{$var}.
        (&variable_conditions_sub): When the call is the top level call,
        generate all the permutations of the conditions.

I don't remember, and I have to leave now.  I'll see at home whether I
have traces about this.  Actually, maybe it is really useless *with
your patch*.

| I'm not going to check this in until I hear from you (even if the
| answer is "I don't know" :-).

Good guess!  :)

Well, I tend to agree with your patch :)

reply via email to

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