dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.88k stars 4.01k forks source link

Compiler does not take all inputs into account for determinism #17121

Open jcouv opened 7 years ago

jcouv commented 7 years ago

In discussion about editorconfig today, a number of scenarios were mentioned that we know or suspect are not properly accounted for in determinism:

FYI @jaredpar

gafter commented 7 years ago

I don't know what you mean by "not properly accounted for". Determinism means that identical inputs result in identical outputs. It does not mean that different inputs result in different outputs. The latter isn't possible (and isn't a goal of the determinism effort), as the output is frequently smaller than the input.

jcouv commented 7 years ago

My understanding is there is a hash embedded in the output file properties, which is computed from various inputs (including content hash for source files). But not all inputs. I'll let @jaredpar confirm or clarify.

jaredpar commented 7 years ago

The other goal of determinism though is content enabled caching. In order to have proper content enabled caching the compiler at minimum needs to have a full documentation of what is part of the compiler input set. I do not believe these items are properly labeled as part of that set.

gafter commented 7 years ago

I understand now.

gafter commented 7 years ago

What about the set of analyzer assemblies?

gafter commented 7 years ago

@jaredpar What is the form of the documentation required to address this issue? Is there an output of the compiler that is the list of things the compilation depended on? Or it is some document somewhere checked into some repository with an English description of the set of inputs that affect the compilation?

jaredpar commented 7 years ago

@gafter it's this file

https://github.com/dotnet/roslyn/blob/master/docs/compilers/Deterministic%20Inputs.md

gafter commented 7 years ago

Rulesets are explicitly documented, as are "(Binary) contents of all files explicitly passed to the compiler, directly or indirectly"

gafter commented 7 years ago

Reopening until editorconfig fallout is clear and documented.

gafter commented 7 years ago

@jaredpar What remains to be done to address this issue?

gafter commented 7 years ago

@jaredpar As far as I know the editorconfig file is not used by the compiler. It may be used by analyzers, as may other files, but we have an umbrella documentation for files used by analyzers.

So I think this work item is complete. Please reopen if you have something specific you believe should be added to the docs.

jaredpar commented 7 years ago

It's an active feature and the design isn't fully closed out.

gafter commented 7 years ago

@jaredpar This issue says that "Compiler does not take all inputs into account for determinism". As far as I know that is not true; it does take all inputs into account. This is not an open design issue, it is a bug report.

If there is another feature that impacts determinism under active development, please open a specific issue against the team doing that development to make sure the team takes determinism into account. As it stands, this issue is too generic and is not actionable.

jasonmalinowski commented 5 years ago

@agocke I see this item is in the IDE tracking project for .editorconfig -- do we know if this bug is even still meaningful at this point or should I close it out?

agocke commented 5 years ago

@jasonmalinowski This bug is still valid, since I'm not sure I updated determinism to account for editorconfig files, but is not IDE-related.