Re: [Jason-bugs] Fw: Anonymous variables are not renamed correctly. (public repost)

[Tim C. <tim...@ya...>]

Jomi,

I have revisited the patch for anonymous variables. This time I have implemented it against the current public svn version 1561. The change to Structure.java is the same (although tidied up a bit). This time, however, I have updated TermTest.java as well. These changes aim not to change the intent of the tests, only the way they are carried out (although I may have made mistakes in identifying their intent, given they aren't named descriptively nor documented). Most of the changes revolve around moving from using toString to testing the objects directly using equals and u.unifies. A side-effect of this is that you can change the way the involved objects are converted to strings without breaking the tests.

The test I had the most difficulty in understanding was testMakeVarAnnon4(). It appears that a unifier is created, a manual renaming of one of the variables is made and it is tested that this manual renaming is respected in makeVarsAnnon (i.e. the variable isn't renamed further). If this is the case, then the new test doesn't really capture that desired behavior. I think, but maybe mistaken, that this behavior is inconsistent with solving my original issue. Just because an unnamed variable hasn't attained a value in the input unifier, does not mean it shouldn't be renamed. Just because a named variable has been renamed to an unnamed variable doesn't mean it does not need further renaming. If you enforce renaming regardless of previous aliasing, then the old renaming must be overridden. Thus, manual aliases cannot be respected. If you ensure that you only rename a variable once given an input unifier then situations where more than one aliasing is
 necessary cannot be accommodated (like that situation identified in my prior email).

There is also an additional test that demonstrates the original problem I was having. The original Structure.java will fail this test. The patched version will not. The only downside to the new Structure.java is that more anonymous variables are created than previously (thus the removal of the count in testMakeVarAnnon3()). Given the original problem was that not enough new variables were created, I fail how to see this would be an issue.

On a related issue, one of the original tests failed (with the new Structure.java) because no unifier was passed into the makeVarsAnnon method. Because makeVarsAnnon modifies the input unifier and an empty default was supplied, the mapping from the old name to the new was lost when the empty default unifier was discarded. Any attempt at unification that does not use a unifier with the mapping from old names to new will fail to assign values to the original variable. Consequently, I believe that it would be beneficial to change the interface of Literal.java to remove the makeVarsAnnon() method. This would make it compulsory to pass in a unifier to this method. Given clients won't know whether a given Literal will create new bindings as a result of anonymizing the variables therein, clients don't know whether to call makeVarsAnnon() or makeVarsAnnon(Unifier u). Most will go with makeVarsAnnon() assuming it will Do the Right Thing, not knowing they may lose
 information about variable aliases.

Hopefully this provides more insight into the issue and that a solution can be arrived at soon.

regards,
Tim


      __________________________________________________________________________________
See what's on at the movies in your area. Find out now: http://au.movies.yahoo.com/session-times/