Skip to content

Improve warning for ignored values in sequence expressions - fixes #4228#4245

Merged
dsyme merged 8 commits into
dotnet:masterfrom
forki:seq
Feb 9, 2018
Merged

Improve warning for ignored values in sequence expressions - fixes #4228#4245
dsyme merged 8 commits into
dotnet:masterfrom
forki:seq

Conversation

@forki

@forki forki commented Jan 23, 2018

Copy link
Copy Markdown
Contributor

fixes #4228

image

@forki

forki commented Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

I used a new error number so that we can built quickfixes on top of it.

@vasily-kirichenko

Copy link
Copy Markdown
Contributor

Please, add type of the ignored expressing into the warning message.

@forki

forki commented Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

good point.

image

@vasily-kirichenko

Copy link
Copy Markdown
Contributor

Excellent, it's in conjunction with the recent Don's PR which added ignored type names into all warning messages.

@forki

forki commented Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

@dsyme this is weird:

src\fsharp\TypeChecker.fs(847,43): error FS0039: The field, constructor or member 'unitTypeExpectedInSequenceExpression' is not defined. 

for src\buildfromsource\FSharp.Compiler.Private\FSharp.Compiler.Private.fsproj in ci_part3 - but it's clearly there

@forki

forki commented Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

@vasily-kirichenko @dsyme another question: I see isListTy and isArrayty but no general isSeqTy. Do we have something like that for TType?

@forki

forki commented Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

@KevinRansom do you see by any chance why CI build 3 does not see the error message?

@KevinRansom

Copy link
Copy Markdown
Contributor

Probably the way that text is converted to resources is not quite wired up the same. I will take a look later today.

@forki

forki commented Jan 23, 2018 via email

Copy link
Copy Markdown
Contributor Author

@KevinRansom

KevinRansom commented Jan 24, 2018

Copy link
Copy Markdown
Contributor

@forki Okay, I pushed a fix.

The reason this happened is because the build failed in the build from source projects. Those guys are for RedHat integration.

That build depends on released product, and the build task that converts from txt file to resx and generates the .fs file is not yet in the build of the dotnet cli that they use. Because of this, we just copy the transformed resx and .fs files. As soon as they update their base build tools we will modify the buildfrom source build to use the buildtask.

Sorry for the inconvenience ...

Kevin

@forki

forki commented Jan 24, 2018

Copy link
Copy Markdown
Contributor Author

ok cool. so basically I need to touch these 2 files as well and things will be fine?

@KevinRansom

Copy link
Copy Markdown
Contributor

@forki, yes, until the build from source uses a recent compiler, then it will be automagic like it should be.

@forki

forki commented Jan 24, 2018

Copy link
Copy Markdown
Contributor Author

ok. this is done.

@cartermp

cartermp commented Feb 5, 2018

Copy link
Copy Markdown
Contributor

ping @KevinRansom @dsyme for review

@dsyme dsyme merged commit 5c491b2 into dotnet:master Feb 9, 2018
@dsyme

dsyme commented Feb 9, 2018

Copy link
Copy Markdown
Contributor

Awesome work, thank you

It's possible a similar improvement could be made for async expressions?

T-Gro pushed a commit that referenced this pull request Jun 12, 2026
 (#4245)

* Improve warning for ignored values in sequence expressions - fixes #4228

* Adding a test

* Show the type

* Add a case for when we need to suggest yield!

* Check sequence type

* Manually copy resx and fs from txt conversion

* fix rebase issue

* Only report yield! for sequences
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.

Improve warning for ignored values in sequence expressions

5 participants