Position/locate/pseudocode revamp (split)

[mahrud]

• removed duplicated keywordTexMath
• renamed thenclause to thenClause in interpreter
• removed functionCode.arrow
• added a frame error in evalraw
• changed how position is stored in Token
• fixed lookup for symbols
• added Pseudocode and PseudocodeClosure types
• removed col+1 from changed tostring(Position)
• removed +1 from endpoint of errors
• added position to stringCode and functionCode
• updated codePosition
• updated convertr.d
• setup functions for some operators
• fixed error start point location in a test
• fixed a ComputationsBook test
• updated documentation for pseudocode
• revamped locate and moved it to debugging.dd
• updated locate and code in code.m2
• moved disassemble and pseudocode to debugging.dd
• added toList(Code) in debugging.dd
• moved tostring(Code) to debugging.dd
• added printing and introspection for Pseudocode
• added typical values for disassemble and pseudocode
• added disassemble for FunctionBody and Pseudocode
• renamed fetch to local-fetch

[mahrud]

@pzinn have a look at this PR. I tried to stay as close to #3363 as possible, but there are several changes:

• I moved tostring(Code) and disassemble to debugging.dd so that even if there's a bug in startup.m2, it is possible to view the microcode.
• I also added disassemble in a few places in the documentation
• I changed a few minor things (e.g fetch to local-fetch) but didn't change anything about printing or your Pseudocode format.

Several things that I think we should consider doing:

1. rename Pseudocode -> PseudocodeBody
2. possibly combine functionBody, symbolBody and have it turn all types of closures (symbol, function, pseudocode) to bodies.
3. add tests!!

To do that last item, I need your help/clarification: I couldn't figure out a precise change resulted by these commits:

• 608a8bd
• f1c880b
• 458809f
• e77b278

Could you give examples of what output these change? I'm guessing two of them might correspond to test fixes in ca8cbbd and 4bb2b0c.

[d-torrance]

For the locate updates, are there any specific examples of things that work better after these changes?

[mahrud]

For the locate updates, are there any specific examples of things that work better after these changes?

Paul mentioned one in the description of his PR, and I added it as a test.

Before:

i1 : () -> "blah
     foo";

i2 : locate oo

o2 = stdio:1:3-1:3

Now:

i1 : () -> "blah
     foo";

i2 : locate oo

o2 = stdio:1:0-2:4

[pzinn]

f1c880b

is about fixing the location of symbols that are "exported" before being defined, as is often done (maybe shouldn't?) in packages. I think we discussed this at some point.

458809f
e77b278

These I think were hacks intended to compensate for the inaccuracy of the position of code. they are no longer needed.

[pzinn]

disassemble and toString now produce almost but not quite the same result on Pseudocode:

i2 : pseudocode(x->x+(1/x))

o2 = function restargs true
              numparms 1
              framesize 1
              frameID 11546
              body 2-OP + local-fetch 0 0
                          2-OP / 1
                                 local-fetch 0 0

o2 : PseudocodeClosure

i3 : disassemble oo

o3 = (function restargs: true numparms: 1 framesize: 1 frameID: 11546 (2-OP + (local-fetch 0 0) (2-OP / 1 (local-fetch
     0 0))))

i4 : toString o2

o4 = (function (restargs true) (numparms 1) (framesize 1) (frameID 11546) (body (2-OP + (local-fetch 0 0) (2-OP / 1
     (local-fetch 0 0)))))

probably toString should just call disassemble.

[mahrud]

is about fixing the location of symbols that are "exported" before being defined, as is often done (maybe shouldn't?) in packages. I think we discussed this at some point.

Can you give an example a situation where this is different now? This didn't work:

i1 : needsPackage "FirstPackage"

o1 = FirstPackage

o1 : Package

i2 : locate firstFunction

i3 : 

[pzinn]

this is about symbol location:

i1 : needsPackage "FirstPackage"

o1 = FirstPackage

o1 : Package

i2 : locate symbol firstFunction

o2 = ../../Macaulay2/packages/FirstPackage.m2:15:0-15:13

o2 : FilePosition

[mahrud]

Thanks, just added a test.

[mahrud]

Getting these new tests to pass has been a pain, but hopefully it'll pay off by preventing accidental regressions.

[mahrud]

Does this have everyone's approval now?

[d-torrance]

I'd still like to spend a bit more time reviewing. Classes just started, but I'll try to get to it this week.

[pzinn]

I'm confused -- what was the point of asking me for a review if it gets merged before I get a chance to complete it?

[d-torrance]

Oops, my fault! Sorry about that

[mahrud]

I'm going to make a new issue to keep track of remaining work. You can still finish the review here too, and I'll add whatever needs to be changed there.

Also, do you know how this "frame error" can occur? 608a8bd

[pzinn]

i1 : f = x -> x+.5

o1 = f

o1 : FunctionClosure

i2 : pseudocode f

o2 = function restargs true
              numparms 1
              framesize 1
              frameID 8943
              body 2-OP + local-fetch 0 0
                          .5

o2 : PseudocodeClosure

i3 : o2_4_1

o3 = .5

o3 : Pseudocode

i4 : value oo

o4 = .5

o4 : RR (of precision 53)

i5 : o2_4_0

o5 = local-fetch 0 0

o5 : Pseudocode

i6 : value oo
stdio:1:9:(3): error: frame error

[mahrud]

I'm confused. In your example I suppose x is not yet defined, but why is this example not working?

i56 : f = x -> y -> x = x+y;

i57 : g = f(0);

i58 : g(1)

o58 = 1

i59 : g(2)

o59 = 3

i60 : g(2)

o60 = 5

i61 : g' = pseudocode g

o61 = function restargs true
               numparms 1
               framesize 1
               frameID 15399
               body local-assign 0 1 2-OP + local-fetch 0 1
                                            local-fetch 0 0

o61 : PseudocodeClosure

i62 : g'_4_0_0

o62 = local-fetch 0 1

o62 : Pseudocode

i63 : value oo
stdio:57:18:(3): error: frame error

In this case x is defined within the frame of g', so shouldn't local-fetch be able to access it?

i64 : peek frame g'

o64 = MutableList{5}

[pzinn]

That's a complicated issue. Yes, in principle, one could try to get a closure even for sub-code of some function, and in fact in some earlier iteration of this PR I did that. But it was too hacky and I gave up on it. Right now Pseudocode (as opposed to PseudocodeClosure) only has the frame that exists at the moment that value is called.

[mahrud]

Ah, I understand, g'_4_0_0 is a Pseudocode not PseudocodeClosure, so it doesn't have the data. Okay that's fine. I wonder if there could be an easy way to manually put together a frame and a pseudocode to get a closure? Like if I did value(frame g', g'_4_0_0) or something like that.

[pzinn]

something like that could work (I hadn't tried that), except the output of frames (preferably with an s) is not the actual (list of) frames but rather some m2-level object that imitates the frames, so it might be a pain to recreate the frame out of it.

[mahrud]

@pzinn what was the motivation behind unseq and the choices for combinePosition instead of treePosition? The positions seem to be random in practice, and I'm thinking about undoing some of these.

[d-torrance]

@mahrud, @pzinn: Do you have a suggestion for how the pseudocode changes should be described in the changelog for the release?