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

Produce ref assemblies from command-line and msbuild #17558

Merged
merged 14 commits into from
Mar 21, 2017
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2288,6 +2288,9 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_Merge_conflict_marker_encountered" xml:space="preserve">
<value>Merge conflict marker encountered</value>
</data>
<data name="ERR_NoRefOutWhenRefOnly" xml:space="preserve">
<value>Do not use refout when using refonly.</value>
</data>
<data name="ERR_OvlOperatorExpected" xml:space="preserve">
<value>Overloadable operator expected</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
string outputDirectory = baseDirectory;
ImmutableArray<KeyValuePair<string, string>> pathMap = ImmutableArray<KeyValuePair<string, string>>.Empty;
string outputFileName = null;
string outputRefFilePath = null;
bool metadataOnly = false;
string documentationPath = null;
string errorLogPath = null;
bool parseDocumentationComments = false; //Don't just null check documentationFileName because we want to do this even if the file name is invalid.
Expand Down Expand Up @@ -379,6 +381,7 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
}

continue;

case "out":
if (string.IsNullOrWhiteSpace(value))
{
Expand All @@ -391,6 +394,26 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar

continue;

case "refout":
value = RemoveQuotesAndSlashes(value);
if (string.IsNullOrEmpty(value))
{
AddDiagnostic(diagnostics, ErrorCode.ERR_NoFileSpec, arg);
}
else
{
outputRefFilePath = ParseGenericPathToFile(value, diagnostics, baseDirectory);
}

continue;

case "refonly":
if (value != null)
break;
Copy link
Member

Choose a reason for hiding this comment

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

Are we testing /refonly:value?


metadataOnly = true;
continue;

case "t":
case "target":
if (value == null)
Expand Down Expand Up @@ -1141,6 +1164,11 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
diagnosticOptions[o.Key] = o.Value;
}

if (metadataOnly && outputRefFilePath != null)
{
AddDiagnostic(diagnostics, diagnosticOptions, ErrorCode.ERR_NoRefOutWhenRefOnly);
}

if (!IsScriptRunner && !sourceFilesSpecified && (outputKind.IsNetModule() || !resourcesOrModulesSpecified))
{
AddDiagnostic(diagnostics, diagnosticOptions, ErrorCode.WRN_NoSources);
Expand Down Expand Up @@ -1257,7 +1285,7 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar

var emitOptions = new EmitOptions
(
metadataOnly: false,
metadataOnly: metadataOnly,
debugInformationFormat: debugInformationFormat,
pdbFilePath: null, // to be determined later
outputNameOverride: null, // to be determined later
Expand All @@ -1282,8 +1310,9 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
Utf8Output = utf8output,
CompilationName = compilationName,
OutputFileName = outputFileName,
OutputRefFilePath = outputRefFilePath,
PdbPath = pdbPath,
EmitPdb = emitPdb,
EmitPdb = emitPdb && !metadataOnly,
Copy link
Member

Choose a reason for hiding this comment

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

emitPdb && !metadataOnly [](start = 26, length = 24)

Code above checks the value of emitPdb. So if we are gonna change it here the check above won't take the metadataOnly flag into account.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's correct and that's by design. The idea is that you can just add /refonly on a command-line and it will work, without having to remove the parameters related to emitting PDBs. For instance, you may have source links and we won't complain about it (one of the checks above).
Does that seem ok?

Copy link
Member

Choose a reason for hiding this comment

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

OK. I'd then add a comment.

SourceLink = sourceLink,
OutputDirectory = outputDirectory,
DocumentationPath = documentationPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2358,8 +2358,6 @@ internal override bool GenerateResourcesAndDocumentationComments(
DiagnosticBag diagnostics,
CancellationToken cancellationToken)
{
Debug.Assert(!moduleBuilder.EmitOptions.EmitMetadataOnly);

// Use a temporary bag so we don't have to refilter pre-existing diagnostics.
var resourceDiagnostics = DiagnosticBag.GetInstance();

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1460,5 +1460,6 @@ internal enum ErrorCode
#endregion more stragglers for C# 7

ERR_Merge_conflict_marker_encountered = 8300,
ERR_NoRefOutWhenRefOnly = 8301,
}
}
108 changes: 108 additions & 0 deletions src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2784,6 +2784,16 @@ public void ParseOut()
// error CS2005: Missing file specification for '/out:' option
Diagnostic(ErrorCode.ERR_NoFileSpec).WithArguments("/out:"));

parsedArgs = DefaultParse(new[] { @"/refout:", "a.cs" }, baseDirectory);
parsedArgs.Errors.Verify(
// error CS2005: Missing file specification for '/refout:' option
Diagnostic(ErrorCode.ERR_NoFileSpec).WithArguments("/refout:"));

parsedArgs = DefaultParse(new[] { @"/refout:ref.dll", "/refonly", "a.cs" }, baseDirectory);
parsedArgs.Errors.Verify(
// error CS8301: Do not use refout when using refonly.
Diagnostic(ErrorCode.ERR_NoRefOutWhenRefOnly).WithLocation(1, 1));

// Dev11 reports CS2007: Unrecognized option: '/out'
parsedArgs = DefaultParse(new[] { @"/out", "a.cs" }, baseDirectory);
parsedArgs.Errors.Verify(
Expand Down Expand Up @@ -8845,6 +8855,104 @@ public void Version()
}
}

[Fact]
public void RefOut()
{
var dir = Temp.CreateDirectory();
dir.CreateDirectory("ref");

var src = dir.CreateFile("a.cs");
src.WriteAllText(@"class C { public static void Main() { } }");

var outWriter = new StringWriter(CultureInfo.InvariantCulture);
var csc = new MockCSharpCompiler(null, dir.Path, new[] { "/nologo", "/out:a.dll", "/refout:ref/a.dll", "/deterministic", "a.cs" });

int exitCode = csc.Run(outWriter);
Assert.Equal(0, exitCode);

var refDll = Path.Combine(dir.Path, "ref\\a.dll");
Copy link
Member

@cston cston Mar 16, 2017

Choose a reason for hiding this comment

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

Path.Combine("ref", "a.dll") for all platforms. #Resolved

Assert.True(File.Exists(refDll));
var refPeStream = File.OpenRead(refDll);

using (var refPeReader = new PEReader(refPeStream))
{
var metadataReader = refPeReader.GetMetadataReader();

// The types and members that are included needs further refinement.
// See issue https://github.com/dotnet/roslyn/issues/17612
AssertEx.SetEqual(metadataReader.TypeDefinitions.Select(t => metadataReader.Dump(t)),
new[] { "TypeDef:<Module>", "TypeDef:C" });

AssertEx.SetEqual(metadataReader.MethodDefinitions.Select(t => metadataReader.Dump(t)),
new[] { "MethodDef: Void Main()", "MethodDef: Void .ctor()" });

// ReferenceAssemblyAttribute is missing.
// See issue https://github.com/dotnet/roslyn/issues/17612
AssertEx.SetEqual(
metadataReader.CustomAttributes.Select(a => metadataReader.GetCustomAttribute(a).Constructor)
.Select(c => metadataReader.GetMemberReference((MemberReferenceHandle)c).Parent)
.Select(p => metadataReader.GetTypeReference((TypeReferenceHandle)p).Name)
.Select(n => metadataReader.GetString(n)),
new[] { "CompilationRelaxationsAttribute", "RuntimeCompatibilityAttribute", "DebuggableAttribute" });

// no method implementations
foreach (var typeDef in metadataReader.TypeDefinitions)
{
Assert.Equal(0, metadataReader.GetTypeDefinition(typeDef).GetMethodImplementations().Count());
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 10, 2017

Choose a reason for hiding this comment

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

Assert.Equal(0, metadataReader.GetTypeDefinition(typeDef).GetMethodImplementations().Count()); [](start = 20, length = 94)

This doesn't actually verify that there were no method bodies emitted, which I assume is the intent here. #Closed

}
}

// Clean up temp files
CleanupAllGeneratedFiles(dir.Path);
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 10, 2017

Choose a reason for hiding this comment

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

CleanupAllGeneratedFiles [](start = 12, length = 24)

We should verify that the normal assembly is what we expect. #Closed

}

[Fact]
public void RefOutWithError()
{
var dir = Temp.CreateDirectory();
dir.CreateDirectory("ref");

var src = dir.CreateFile("a.cs");
src.WriteAllText(@"class C { public static void Main() { error(); } }");

var outWriter = new StringWriter(CultureInfo.InvariantCulture);
var csc = new MockCSharpCompiler(null, dir.Path, new[] { "/nologo", "/out:a.dll", "/refout:ref/a.dll", "/deterministic", "a.cs" });
int exitCode = csc.Run(outWriter);
Assert.Equal(1, exitCode);
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 10, 2017

Choose a reason for hiding this comment

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

Assert.Equal(1, exitCode); [](start = 12, length = 26)

It would be great to examine the diagnostics. #Closed


var dll = Path.Combine(dir.Path, "a.dll");
Assert.False(File.Exists(dll));

var refDll = Path.Combine(dir.Path, "ref\\a.dll");
Assert.True(File.Exists(refDll));

// Clean up temp files
CleanupAllGeneratedFiles(dir.Path);
}

[Fact]
public void RefOnly()
{
var dir = Temp.CreateDirectory();

var src = dir.CreateFile("a.cs");
src.WriteAllText(@"class C { public static void Main() { error(); } }"); // semantic error in method body

var outWriter = new StringWriter(CultureInfo.InvariantCulture);
var csc = new MockCSharpCompiler(null, dir.Path, new[] { "/nologo", "/out:a.dll", "/refonly", "/debug", "/deterministic", "a.cs" });
int exitCode = csc.Run(outWriter);
Assert.Equal(0, exitCode);
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 10, 2017

Choose a reason for hiding this comment

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

Assert.Equal(0, exitCode); [](start = 12, length = 26)

I think we should verify content of a.dll against our expectations. #Closed


var refDll = Path.Combine(dir.Path, "a.dll");
Assert.True(File.Exists(refDll));

var pdb = Path.Combine(dir.Path, "a.pdb");
Assert.False(File.Exists(pdb));

// Clean up temp files
CleanupAllGeneratedFiles(dir.Path);
}

public class QuotedArgumentTests
{
private void VerifyQuotedValid<T>(string name, string value, T expected, Func<CSharpCommandLineArguments, T> getValue)
Expand Down
92 changes: 85 additions & 7 deletions src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void Main()
EmitResult emitResult;
using (var output = new MemoryStream())
{
emitResult = compilation.Emit(output, null, null, null);
emitResult = compilation.Emit(output, pdbStream: null, xmlDocumentationStream: null, win32Resources: null);
}

emitResult.Diagnostics.Verify(
Expand Down Expand Up @@ -148,7 +148,7 @@ namespace N.Foo;
EmitResult emitResult;
using (var output = new MemoryStream())
{
emitResult = comp.Emit(output, null, null, null);
emitResult = comp.Emit(output, pdbStream: null, xmlDocumentationStream: null, win32Resources: null);
}

Assert.False(emitResult.Success);
Expand Down Expand Up @@ -236,6 +236,84 @@ public static void Main()
}
}

[Theory,
InlineData("public int M() { return 1; }", "public int M() { return 2; }", true),
InlineData("public int M() { return 1; }", "public int M() { error(); }", true),
InlineData("", "private void M() { }", false), // Should be true. See follow-up issue https://github.com/dotnet/roslyn/issues/17612
InlineData("internal void M() { }", "", false),
InlineData("public struct S { private int i; }", "public struct S { }", false)]
public void RefAssemblyChanges(string change1, string change2, bool expectMatch)
{
string sourceTemplate = @"
public class C
{
CHANGE
}
";
string name = GetUniqueName();
string source1 = sourceTemplate.Replace("CHANGE", change1);
CSharpCompilation comp1 = CreateCompilationWithMscorlib(Parse(source1),
options: TestOptions.DebugDll.WithDeterministic(true), assemblyName: name);

byte[] image1;

using (var output = new MemoryStream())
Copy link
Member

@cston cston Mar 17, 2017

Choose a reason for hiding this comment

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

output is not used. #Resolved

using (var pdbOutput = new MemoryStream())
{
var emitResult = comp1.Emit(output, pdbOutput, options: EmitOptions.Default.WithEmitMetadataOnly(true));
Assert.True(emitResult.Success);
emitResult.Diagnostics.Verify();
image1 = output.ToArray();
}
Copy link
Member

@cston cston Mar 17, 2017

Choose a reason for hiding this comment

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

var image1 = comp1.EmitToArray(EmitOptions.Default.WithEmitMetadataOnly(true)); #Resolved


var source2 = sourceTemplate.Replace("CHANGE", change2);
Compilation comp2 = CreateCompilationWithMscorlib(Parse(source2),
options: TestOptions.DebugDll.WithDeterministic(true), assemblyName: name);

byte[] image2;

using (var output = new MemoryStream())
using (var pdbOutput = new MemoryStream())
{
var emitResult = comp2.Emit(output, pdbOutput, options: EmitOptions.Default.WithEmitMetadataOnly(true));
Assert.True(emitResult.Success);
emitResult.Diagnostics.Verify();
image2 = output.ToArray();
}

Assert.True(AssertEx.SequenceEqual(image1, image2) == expectMatch);
}

[Theory,
InlineData("public int M() { error(); }", true),
InlineData("public int M() { error() }", false), // Should be true. See follow-up issue https://github.com/dotnet/roslyn/issues/17612
InlineData("public Error M() { return null; }", false), // Should be true. See follow-up issue https://github.com/dotnet/roslyn/issues/17612
]
public void RefAssemblyDiagnostics(string change, bool expectSuccess)
{
string sourceTemplate = @"
public class C
{
CHANGE
}
";
string source = sourceTemplate.Replace("CHANGE", change);
string name = GetUniqueName();
CSharpCompilation comp1 = CreateCompilationWithMscorlib(Parse(source),
options: TestOptions.DebugDll.WithDeterministic(true), assemblyName: name);

byte[] image;

using (var output = new MemoryStream())
using (var pdbOutput = new MemoryStream())
{
var emitResult = comp1.Emit(output, pdbOutput, options: EmitOptions.Default.WithEmitMetadataOnly(true));
Assert.Equal(expectSuccess, emitResult.Success);
Assert.Equal(!expectSuccess, emitResult.Diagnostics.Any());
image = output.ToArray();
}
}

/// <summary>
/// Check that when we emit metadata only, we include metadata for
/// compiler generate methods (e.g. the ones for implicit interface
Expand Down Expand Up @@ -939,7 +1017,7 @@ public class Test
EmitResult emitResult;
using (var output = new MemoryStream())
{
emitResult = compilation.Emit(output, null, null, null);
emitResult = compilation.Emit(output, pdbStream: null, xmlDocumentationStream: null, win32Resources: null);
}

Assert.False(emitResult.Success);
Expand Down Expand Up @@ -972,7 +1050,7 @@ public static void Main()
EmitResult emitResult;
using (var output = new MemoryStream())
{
emitResult = compilation.Emit(output, null, null, null);
emitResult = compilation.Emit(output, pdbStream: null, xmlDocumentationStream: null, win32Resources: null);
}

Assert.True(emitResult.Success);
Expand Down Expand Up @@ -1010,7 +1088,7 @@ public static void Main()
EmitResult emitResult;
using (var output = new MemoryStream())
{
emitResult = compilation.Emit(output, null, null, null);
emitResult = compilation.Emit(output, pdbStream: null, xmlDocumentationStream: null, win32Resources: null);
}

Assert.True(emitResult.Success);
Expand Down Expand Up @@ -2726,7 +2804,7 @@ public void BrokenPDBStream()
var output = new MemoryStream();
var pdb = new BrokenStream();
pdb.BreakHow = BrokenStream.BreakHowType.ThrowOnSetLength;
var result = compilation.Emit(output, pdb);
var result = compilation.Emit(output, pdbStream: pdb);

// error CS0041: Unexpected error writing debug information -- 'Exception from HRESULT: 0x806D0004'
var err = result.Diagnostics.Single();
Expand All @@ -2737,7 +2815,7 @@ public void BrokenPDBStream()
Assert.Equal(ioExceptionMessage, (string)err.Arguments[0]);

pdb.Dispose();
result = compilation.Emit(output, pdb);
result = compilation.Emit(output, pdbStream: pdb);

// error CS0041: Unexpected error writing debug information -- 'Exception from HRESULT: 0x806D0004'
err = result.Diagnostics.Single();
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Emit/Emit/DeterministicTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ public void BanVersionWildcards()
references: new[] { MscorlibRef },
options: TestOptions.DebugDll.WithDeterministic(false));

var resultDeterministic = compilationDeterministic.Emit(Stream.Null, Stream.Null);
var resultNonDeterministic = compilationNonDeterministic.Emit(Stream.Null, Stream.Null);
var resultDeterministic = compilationDeterministic.Emit(Stream.Null, pdbStream: Stream.Null);
var resultNonDeterministic = compilationNonDeterministic.Emit(Stream.Null, pdbStream: Stream.Null);

Assert.False(resultDeterministic.Success);
Assert.True(resultNonDeterministic.Success);
Expand Down
Loading