tools.analyzer

Update the namespace in the env on each recursive all to analyze+eval

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

Without a change similar to the one in the attached patch, I do not see how to get the correct namespace associated with some warnings in Eastwood, particularly the ones for :unlimited-use. The value bound to ns does change as soon as an in-ns form is evaluated, which is in the middle of handling the Gilardi scenario.

  1. tanal-80-v1.diff
    12/Jun/14 6:02 AM
    3 kB
    Andy Fingerhut
  2. tanal-80-v2.diff
    12/Jun/14 6:37 AM
    2 kB
    Andy Fingerhut

Activity

Hide
Andy Fingerhut added a comment -

Patch tanal-80-v1.diff is one modification I have tested, and it seems to do the job. With this, Eastwood passes its few unit tests, and the crucible test results, while not 100% identical to what they were before using ana.jvm/analyze+eval, are very very close.

The introduction of butlast+last is not a functional change, but merely giving name to some code inside analyze+eval and defining it separately. Certainly not a necessary part of the desired change, but I thought I'd suggest it.

Show
Andy Fingerhut added a comment - Patch tanal-80-v1.diff is one modification I have tested, and it seems to do the job. With this, Eastwood passes its few unit tests, and the crucible test results, while not 100% identical to what they were before using ana.jvm/analyze+eval, are very very close. The introduction of butlast+last is not a functional change, but merely giving name to some code inside analyze+eval and defining it separately. Certainly not a necessary part of the desired change, but I thought I'd suggest it.
Hide
Nicola Mometto added a comment -

Is there any reason why you replaced the mapv with a loop/recur? I don't think there's any behavioural difference and if so can you update the patch to switch back to mapv?

Otherwise the patch looks reasonable and I'm eager to apply it.

Show
Nicola Mometto added a comment - Is there any reason why you replaced the mapv with a loop/recur? I don't think there's any behavioural difference and if so can you update the patch to switch back to mapv? Otherwise the patch looks reasonable and I'm eager to apply it.
Hide
Andy Fingerhut added a comment -

tanal-80-v2.diff is same as -v1, but switches loop back to mapv.

Show
Andy Fingerhut added a comment - tanal-80-v2.diff is same as -v1, but switches loop back to mapv.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: