Conversation
| List.revIter (Binding >> add) bindings | ||
| // let! or use! | ||
| | SynExpr.LetOrUse(_, _, _, true, bindings, leftHandSide, _, _) -> | ||
| match bindings with |
There was a problem hiding this comment.
Change based on the FCS release notes at https://github.com/dotnet/fsharp/blob/c1b13e3d4542d1de7439b34de00384934365aeb9/docs/release-notes/.FSharp.Compiler.Service/10.0.100.md?plain=1#L63
Numpsy
force-pushed
the
fcs10
branch
2 times, most recently
from
e5c21fc to
032f006
Compare
Numpsy
force-pushed
the
fcs10
branch
3 times, most recently
from
6b9014d to
6ab840a
Compare
|
Not sure why the docs build failed with the build using the RTM FCS 43.10 when it worked with the RC2 version |
|
Or is that just because the CI has started installing .NET 10 on it's own when it didn't this morning? 
Apparently the use-dotnet action always installs the latest version now, which is .NET 10 as of earlier today - actions/setup-dotnet#674 |
|
@Numpsy you're right, our daily CI run has started to fail today. Maybe setup-dotnet doesn't honor |
|
@Numpsy actually, I don't think it's a bug in setup-dotnet action because all other CI jobs work except the docs one. So maybe it's a bug in Fornax? Fornax not honoring global.json? |
|
It sounds like it's running with the .NET 9 SDK but trying to build as .NET 10 for some reason? (though the error shows the 9.0.306 SDK, and global.json is 9.0.201). |
There does seem to be some machinery at https://github.com/ionide/Fornax/blob/master/src/Fornax/FSIRefs.fs for fishing for runtime libraries, so maybe you're right there and something is going wrong with .NET 10 |
|
I tried changing the CI to actually run as .NET 10 and that fails with a type check error in the build script - https://github.com/Numpsy/FSharpLint/actions/runs/19300021169/job/55193408198#step:7:12 Nothing can ever be simple :-( |
knocte
changed the title
Hey @Numpsy I probably managed to fix the above problem with this commit from my fork: knocte@3cc0a65 can you include it in this PR and rebase? |
|
I'll have a look later |
Numpsy
force-pushed
the
fcs10
branch
2 times, most recently
from
dc7abab to
8bf0fa8
Compare
| fieldPats | ||
| |> List.map (fun ((_, fieldIdent), _, _) -> fieldIdent.idRange) | ||
| |> List.map (fun patPairField -> patPairField.Range) | ||
| // @@TODO@@ Do we need to look at the ranges inside FieldName here? |
There was a problem hiding this comment.
Any comments on this one?
There was a problem hiding this comment.
Not sure TBH, but if unit tests pass without looking at the ranges, then I wouldn't oppose merging this PR before we investigate this. That said, if you can come up with a unit test that can exercise this code, then I'd sleep better. BTW, I'm aiming to merge this PR when FCS 43.10.103 gets released (because that version will contain a fix which now is only in their master branch).
There was a problem hiding this comment.
Ok.
FWIW, 43.10.102 seems to work without any further code changes.
Numpsy
changed the title
Numpsy
force-pushed
the
fcs10
branch
2 times, most recently
from
d1cadc0 to
aaba91d
Compare
Numpsy
changed the title
…eing too long" This reverts commit de27b1b.
|
@Numpsy thanks for keeping an eye on FCS versions! I was glad today when I saw your .103 commit, however:
|
|
I was just trying to have a look at that :-( The build at https://github.com/fsprojects/FSharpLint/actions/runs/21730690753 used 43.10.100 and ran in the usual amount of time. The build at https://github.com/fsprojects/FSharpLint/actions/runs/21876176125 which was also 43.10.100 but rebased on top of #841 was terribly slow at running the self-lint The only local change in here between those two was this one 
So it doesn't seem like FCS 10 on its own caused an issue, but it looks like there's some bad interaction between that and the other recent changes |
|
I ran the latest code on a profiler and got this: 
Which I guess is coming from https://github.com/webwarrior-ws/FSharpLint/blob/c378757ad49d9a0c9eedbf228ced722245643978/src/FSharpLint.Core/Rules/Conventions/Naming/AsynchronousFunctionNames.fs#L36 which was recently added? With the current FCS9 it gets this, which is still a hotspot in the same place, but doesn't take nearly as much time 
I've seen a number of bugs raised in the F# repo about this cache stuff causing performance issues of which some are fixed and some which aren't, though I'm sure I've read of issues elsewhere related to getting those project contexts lots of times but I can't recall the details. |
|
Would something like Numpsy@11e75ec where it only creates the ProjectOptions once help? Ci build for that - https://github.com/Numpsy/FSharpLint/actions/runs/21925277621 That's a bit more like how it's done in the analyzers sdk, where the checker results and the project options are both passed down from the top - https://github.com/ionide/FSharp.Analyzers.SDK/blob/294cae109f63a585c35a6e3e2f196358e248371e/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fsi#L106 |
2 participants
Just having a go at building it with the RC .NET 10 version to see if it all works.