Skip to content

Conversation

@0101
Copy link
Contributor

@0101 0101 commented Jan 6, 2023

Fixes #14277

  1. We add OtherRange option to ArgReprInfo and add ArgReprInfo option to ValOptionalData. Then we're able to propagate signature location from SignatureConformance to the parameter Vals in lambdas.

  2. With this we can tell we're looking at a parameter which is also in a signature file when processing Symbol.IsPrivateToFile and new Symbol.IsPrivateToFileAndSignatureFile

  3. We can use Symbol.IsPrivateToFileAndSignatureFile in VS when finding references to only look at the two corresponding files instead of going through all of them.


This still doesn't work for method parameters, because the OtherRange is not propagated to the correct instance of ArgReprInfo there for some reason. Created a separate issue for that: #14753. But at least renaming the parameter in signature file no longer produces an error dialog.

@0101
Copy link
Contributor Author

0101 commented Jan 16, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

I've discussed with @0101 . One more change is needed (more to cenv not TcEnv) but otherwise this is fine. Marking as approved.

@0101
Copy link
Contributor Author

0101 commented Apr 14, 2023

Ok, so after moving ArgInfoCache to cenv the flakiness returned to compressed metadata builds.

I couldn't entirely pinpoint where the issue originates, but it did go away when I cleared FSharpChecker cache before getting the type checking results where we test the symbol.

It also goes away when we don't auto-generate signatures with the ProjectGeneration framework:

match file.SignatureFile with
| AutoGenerated when generateSignatureFiles ->
let project = { p with SourceFiles = p.SourceFiles[0..i] }
let! results = checkFile file.Id project checker
let signature = getSignature results
writeFileIfChanged signatureFileName signature

So something sketchy is probably happening either in IncrementalBuilder or in ProjectGeneration. Either way that shouldn't be affected by this PR. So I propose to adjust the tests to avoid the problem (removing auto-generation of signatures, which is the likely culprit) and merging it.


The cache in cenv is still useful to enable the scenario for method parameters which doesn't work without it. In case of function parameters it's never hit.

@dsyme
Copy link
Contributor

dsyme commented Apr 14, 2023

So I propose to adjust the tests to avoid the problem (removing auto-generation of signatures, which is the likely culprit) and merging it.

Seems very reasonable!

@0101 0101 enabled auto-merge (squash) April 14, 2023 17:04
@0101 0101 merged commit 8a6a476 into dotnet:main Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

5 participants