Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Fix some errors reported by xlab #2991

Merged
merged 3 commits into from
Apr 21, 2020
Merged

Fix some errors reported by xlab #2991

merged 3 commits into from
Apr 21, 2020

Conversation

haltman-at
Copy link
Contributor

@haltman-at haltman-at commented Apr 21, 2020

This PR fixes some problems reported by @xlab in #2977.

The first of these is that having nonconsecutive sources can crash the debugger. I fixed both the part that xlab pointed out, and another point in the code I found that could have trouble on nonconsecutive sources (hopefully these were indeed the only such spots).

The second of these is a bit more mysterious: If the check that xlab pointed out was fixed, the debugger would hang on startup. On investigation, I determined that there were two particular ASTs that would never finish walking. I was unable to determine any reason for this. However, I replaced the uses of yield call(walk, ...) in the AST saga with a simple yield* walk(...), and this somehow caused the problem to go away. (No, we're not somehow losing parallelism here; yield call is completely synchronous.) I have no idea why; some sort of weird redux-saga internal thing got overloaded from all the effects, maybe? IDK. But this fixed the problem, so, uh, I'm sticking with it.

Finally, I altered the CLI logic to force a recompile if source indices are not consecutive. Now, you may recall that the first commit in this PR fixes problems caused by nonconsecutive indices, so why do this? Because nonconsecutive sources, while they should no longer cause a problem, still indicate a problem. In the case of xlab's repo, the nonconsecutivity was due to name collisions, which Truffle infamously can't really handle. Or at least, Truffle's artifacts can't handle this, but the raw compiler output can, so by forcing a recompile we avoid the problem. (At the cost of a lot of time.)

@xlab
Copy link

xlab commented Apr 21, 2020

some sort of weird redux-saga internal thing got overloaded from all the effects

Oh my! Thanks for the investigation and improvements, this was a really deep rabbit hole. Will wait for Truffle DB to evolve ;)

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good digging @haltman-at, and thanks @xlab for the bug report!

@haltman-at haltman-at merged commit 98bc63a into develop Apr 21, 2020
@haltman-at haltman-at deleted the no-source branch April 21, 2020 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants