Skip to content

Type checker: don't suppress errors while checking expressions #18311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Apr 9, 2025

Conversation

auduchinok
Copy link
Member

Fixes #17787.

I guess many tests may need to be updated. 🙂

@auduchinok auduchinok requested a review from a team as a code owner February 12, 2025 17:41
Copy link
Contributor

github-actions bot commented Feb 12, 2025

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/9.0.300.md

@psfinaki
Copy link
Member

Well - 11 tests, that's much less than I would have expected here :)

@auduchinok auduchinok force-pushed the tcExpr-noErrorSuppress branch from e025765 to a8c081a Compare February 20, 2025 18:32
@auduchinok
Copy link
Member Author

auduchinok commented Mar 13, 2025

Could someone please help me with updating the baselines here? I've tried committing the files produced during the local tests run, but it doesn't seem to succeed on CI.

@Martin521
Copy link
Contributor

Martin521 commented Mar 13, 2025

This does not look like a baseline issue, all the CI runs today had these failures.
See also here.

EDIT: sorry, forget that, seems to be different

@auduchinok
Copy link
Member Author

Could someone please help me with updating the baselines here? I've tried committing the files produced during the local tests run, but it doesn't seem to succeed on CI.

@psfinaki
Copy link
Member

psfinaki commented Apr 3, 2025

Hi @auduchinok sorry for the long delay here. I finally got to this - I pushed the changes and it's all green now.

You were actually close. The thing is - there are 2 kinds of baselines here, .bsl and .vsbsl - and this test required adding the latter. Don't ask me why it's like this... artifacts of earlier testing approaches, you know.

@auduchinok
Copy link
Member Author

@psfinaki Thank you very much!

@auduchinok
Copy link
Member Author

auduchinok commented Apr 8, 2025

The tests seem to fail because of #4516. I wish we could finally fix it and not create workarounds in the tooling and tests.

@auduchinok
Copy link
Member Author

This is ready.

There was some difference between the local run and the CI in how the overload resolution error is reported. This could be because of how FCS tests build project options, which may be somewhat different from the CI. I've removed the diagnostic messages from the new tests for now.

@T-Gro T-Gro enabled auto-merge (squash) April 9, 2025 13:00
Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

Great job, thanks!

@T-Gro T-Gro merged commit be2cd93 into dotnet:main Apr 9, 2025
33 checks passed
@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Apr 9, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in F# Compiler and Tooling Apr 9, 2025
@auduchinok auduchinok deleted the tcExpr-noErrorSuppress branch April 9, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Type checker errors should be reported when syntax errors are present
5 participants