Skip to content
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

Symbol.Locations has indeterministic ordering of locations between compilations with same inputs #11015

Closed
mavasani opened this issue May 2, 2016 · 17 comments
Assignees
Labels
Area-Compilers Bug Concept-API This issue involves adding, removing, clarification, or modification of an API. Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time.
Milestone

Comments

@mavasani
Copy link
Contributor

mavasani commented May 2, 2016

See #8067 (comment) for a repro.

This will cause inconsistent analyzer diagnostics across command builds (most symbol analyzers will report diagnostic on the first location of the symbol). Additionally, it is already causing a race condition in IDE diagnostic engine, that uses different compilations to compute open file and full project diagnostics, to miss out on reporting some analyzer diagnostics.

@jaredpar
Copy link
Member

jaredpar commented May 3, 2016

Is this in the analyzer layer or compiler?

@mavasani
Copy link
Contributor Author

mavasani commented May 3, 2016

This is compiler layer (symbol.Locations API) - but the impact seems to be for the end users that install analyzers.

@jaredpar
Copy link
Member

jaredpar commented May 3, 2016

Is this just an error presentation problem? The mention of a "race in the IDE" seems like it could be bad (crashes scary) or just changes order of errors (punt to next version).

@mavasani
Copy link
Contributor Author

mavasani commented May 3, 2016

End user will see different set of diagnostics based on which files are open/closed. Additionally, the location (file path) for analyzer diagnostics reported on partial types will vary every time the intellisense build kicks off (say editing any top level declarations). I think we ought to fix this for Update3.

@mavasani
Copy link
Contributor Author

mavasani commented May 3, 2016

Also tagging @heejaechang @srivatsn for discussion.

@jaredpar
Copy link
Member

jaredpar commented May 3, 2016

@mavasani unfortunately diagnostics don't meet the bar for Update 3 right now.

@jaredpar jaredpar modified the milestones: 2.0 (RC), 1.3 May 3, 2016
@mavasani
Copy link
Contributor Author

mavasani commented May 3, 2016

@heejaechang @srivatsn - I think we need to put a workaround in the IDE diagnostic engine until compiler fixes this bug. Intellisense diagnostics from full solution analysis are essentially broken if we do nothing here.

@heejaechang
Copy link
Contributor

just talked with @mavasani , I think it might not possible to workaround it in IDE side.

@mavasani
Copy link
Contributor Author

mavasani commented May 3, 2016

@jaredpar I am willing to work on a fix in the compiler layer, if the compiler team will take this for Update3.

@jaredpar
Copy link
Member

jaredpar commented May 4, 2016

@mavasani it's less about who does the work and more about we're avoiding any changes to the compiler that don't meet our bar for stability reasons. As explained here it just seems like this is a diagnostic ordering issue which is well below our bar. Is there something I'm missing that makes it more compelling?

@mavasani
Copy link
Contributor Author

mavasani commented May 4, 2016

Yes, there seems to be a misunderstanding. This is not an ordering issue, its a functionality issue. You will see different number of intellisense diagnostics in the error list based on which files are open/closed. Additionally, the file path/location of reported diagnostics will vary from compilation to compilation - i.e. basically as you type, analyzer diagnostic reported on a partial type can jump from one partial definition to another, and squiggles will come and go. Essentially, error list and squiggles will be indeterministic for all analyzer diagnostics from symbol analyzers.

@srivatsn
Copy link
Contributor

srivatsn commented May 4, 2016

Full solution analysis being off by default exacerbates this. Before, at least if the error list was open, you'd see the error from the other partials - just the squiggles would be different . Now with fsd off, if the other partial definition is not open, the error will come and go seemingly randomly.

@jaredpar
Copy link
Member

jaredpar commented May 4, 2016

Gotcha. I agree the scenario meets the bar for U3

@jaredpar jaredpar modified the milestones: 1.3, 2.0 (RC) May 4, 2016
@gafter gafter added Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. Concept-API This issue involves adding, removing, clarification, or modification of an API. labels May 5, 2016
@mavasani
Copy link
Contributor Author

#3748 is also affected due to this bug.

@weltkante
Copy link
Contributor

@cston @mavasani this issue being closed, whats the status of original issue #8067 (which is still on blocked) ?

@mavasani
Copy link
Contributor Author

mavasani commented Jun 9, 2016

I'll verify it tomorrow

@mavasani
Copy link
Contributor Author

@weltkante I have verified that #8067 is now fixed on latest bits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Concept-API This issue involves adding, removing, clarification, or modification of an API. Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time.
Projects
None yet
Development

No branches or pull requests

7 participants