Skip to content

Conversation

@dwijnand
Copy link
Member

Fixes #1023

@dwijnand dwijnand marked this pull request as ready for review March 23, 2022 08:48
@dwijnand dwijnand requested a review from smarter March 23, 2022 08:48
@dwijnand dwijnand requested review from nicolasstucki and removed request for smarter March 23, 2022 16:42
@nicolasstucki
Copy link
Contributor

I wouldn't say that this fixes issue #1023, it is rather a first step towards having this infrastructure.

Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

We should add tests for many of the combinations we want to catch.

One way is to create those trees with macros and let the Ycheck catch them. The issue is that this would end up in a crash of the compiler. We could start improving Ychecking and emit some kind of fatal errors instead. These errors should include the stack trace and explicitly mention that this was an unexpected state in the tree. The unexpected state in the tree could be due to a compiler bug or a bug in a macro.

@dwijnand dwijnand assigned dwijnand and unassigned nicolasstucki Apr 7, 2022
dwijnand added 3 commits April 7, 2022 15:32
Now that we're not running this on every `typed` invocation, the
compilation unit tree isn't going to be a top-level TypeTree.
I'm not sure what I had hit that lead me to this exclusion, but I'm not
hitting now..
@dwijnand dwijnand assigned nicolasstucki and unassigned dwijnand Apr 7, 2022
@nicolasstucki nicolasstucki dismissed smarter’s stale review April 20, 2022 14:08

Outdated review

@nicolasstucki nicolasstucki merged commit 0f6bcd3 into scala:main Apr 20, 2022
@dwijnand dwijnand deleted the fix-#1023 branch April 20, 2022 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ycheck that trees are in place where they make sense

3 participants