-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Rebuild support for OutputKind #52340
Conversation
df6b6ab
to
d47668f
Compare
@dotnet/roslyn-compiler PTAL |
The `OutputKind` value was not properly round tripping through the PDB. Instead we were only distinguishing between roughly exe and dll outputs. The compiler though knows about six different output kinds and that both impacts the manner in which compilation takes place as well as what values are encoded into the PDB (impacts the `Subsystem` value for example). Properly round tripping the output kind here.
src/Compilers/Core/RebuildTest/CompilationOptionsReaderTests.cs
Outdated
Show resolved
Hide resolved
|
||
return list; | ||
|
||
IEnumerable<CommandInfo> Permutate(CommandInfo commandInfo) | ||
void Permutate(CommandInfo commandInfo, params Func<CommandInfo, IEnumerable<CommandInfo>>[] permutations) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a little nervous that we are going to find out about n-factorial eventually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not gotta lie, I screwed this up once and was pleasantly surprised at how well test explorer scaled to my massive mistake
@@ -37,5 +37,6 @@ internal static class CompilationOptionNames | |||
public const string OptionInfer = "option-infer"; | |||
public const string OptionExplicit = "option-explicit"; | |||
public const string OptionCompareText = "option-compare-text"; | |||
public const string OutputKind = "output-kind"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
output [](start = 42, length = 6)
Can't the exact kind be inferred from the headers? It's not ideal when we duplicate information that's already there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was unable to find a way to do this. If you can see a way I'm happy to flip to using that but there didn't seem to be the necessary info in the headers for all of the output kinds that we support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, there is no way to distinguish winmdobj vs library. Everything else could be distinguished.
- OutputKind.ConsoleApplication: Subsystem.WindowsCui
- OutputKind.WindowsApplication: Subsystem.WindowsGui
- OutputKind.WindowsRuntimeApplication: Subsystem.WindowsGui && DllCharacteristics.AppContainer
- OutputKind.WindowsRuntimeMetadata: Characteristics.Dll && IsAssembly
- OutputKind.DynamicallyLinkedLibrary: Characteristics.Dll && IsAssembly
- OutputKind.NetModule: Characteristics.Dll && !IsAssembly
These two command lines result in the same output:
csc /target:winmdobj a.cs /out:1\a.dll /deterministic /subsystemversion:1.0
csc /target:library a.cs /out:2\a.dll /deterministic /subsystemversion:1.0
BTW: would be good to keep https://github.com/dotnet/roslyn/blob/main/docs/features/pdb-compilation-options.md#csharp-flags-that-can-be-derived-from-pdb-or-assembly up to date
In reply to: 608824123 [](ancestors = 608824123)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad. I forgot we had put so much detail into that doc. It's out of date for a lot of changes I made. I will send a PR to get this current.
The
OutputKind
value was not properly round tripping through the PDB. Insteadwe were only distinguishing between roughly exe and dll outputs. The compiler
though knows about six different output kinds and that both impacts the manner
in which compilation takes place as well as what values are encoded into the
PDB (impacts the
Subsystem
value for example). Properly round tripping theoutput kind here.
This builds off #52321 and will be kept in draft until that PR is complete.