Skip to content

Fix #33732 - #36077

Merged
Nathan Shively-Sanders (sandersn) merged 3 commits into
microsoft:masterfrom
jack-williams:fix33732
Mar 17, 2020
Merged

Fix #33732#36077
Nathan Shively-Sanders (sandersn) merged 3 commits into
microsoft:masterfrom
jack-williams:fix33732

Conversation

@jack-williams

Copy link
Copy Markdown
Collaborator

Fixes#33732

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm pretty sure I helped suggest what ultimately became this change, which is why I requested someone else's review on it initially. So naturally, I'm OK with it. But someone else should really review this and try to understand why this fixes the bug without breaking anything.

@arcanis

Copy link
Copy Markdown
Contributor

For what it's worth, our codebase (which exhibited this issue in 3.6+, and seemed to be resolved by merging this PR) no longer triggers this assertion with 3.8.

@jack-williams

Copy link
Copy Markdown
CollaboratorAuthor

Yes, it appears that something in 3.8.?? fixes this, but it still breaks in 3.7.?? and below.

Do we want to patch older versions (so we should keep this fix).
If not, I can remove the change but I suggest keeping the test for regressions.

@sandersn

Copy link
Copy Markdown
Member

I'm pretty sure Anders Hejlsberg (@ahejlsberg) shipped a fix or two to union assignability in 3.8. He definitely encountered this assert along the way, so I think the improvement in 3.8 is intentional. Let's trim this PR down to the test.

The assertion should happen only in cases where the compiler previously failed to give an error when it should. I guess I'd rather have the assert than a failure to error, and in any case we would only ship an extremely urgent fix to 3.7. I don't think that #33732 is that urgent.

@sandersn
Nathan Shively-Sanders (sandersn) merged commit c600aa7 into microsoft:masterMar 17, 2020
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

For Backlog BugPRs that fix a backlog bug

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Error: Debug Failure. False expression: parameter should have errors when reporting errors

5 participants

@jack-williams@arcanis@sandersn@weswigham@RyanCavanaugh