search

racket / drracket

DrRacket, IDE for Racket
http://www.racket-lang.org/
Other
447 stars 94 forks source link

stack-checkpoint: evaluate ccm at the correct context #459

Closed sorawee closed 3 years ago

sorawee commented 3 years ago

The current code has an off-by-one error in a sense. It evaluates ccm at a wrong context, causing the remaining stack after cutting the checkpoint to be unclean (notice the line call-with-stack-checkpoint in the below "unpatched" screenshots). This PR fixes the issue.

Racket CS (patched):

Screen Shot 2021-02-02 at 9 10 52 AM

Racket CS (unpatched)

Screen Shot 2021-02-02 at 9 09 57 AM

Racket BC (patched)

Screen Shot 2021-02-02 at 9 12 29 AM

Racket BC (unpatched)

Screen Shot 2021-02-02 at 9 13 22 AM
rfindler commented 3 years ago

Nice, thanks! I'll want to run the drracket test suites to see if any of the tests that look at stacks change (and if not, maybe add something that checks to make sure that stack-checkpoint.rkt doesn't appear on the stack, maybe?).

rfindler commented 3 years ago

None of the DrRacket test suites seem to detect this change, so I've pushed the commit along with a test case in the file.

sorawee commented 3 years ago

@rfindler FYI: GitHub shows me that https://github.com/racket/drracket/commit/171379b470a99bf9ec8d985028ee7993b9371c49 is an empty commit.

rfindler commented 3 years ago

Oops! I've tried again. Sorry about that; I'm not sure what I did.