Skip to content

Commit

Permalink
Address feedback from Tomas on dotnet#17558
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed Mar 23, 2017
1 parent da63887 commit b360fd0
Show file tree
Hide file tree
Showing 14 changed files with 232 additions and 210 deletions.
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CommandLine/CSharpCommandLineParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1317,7 +1317,7 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
OutputFileName = outputFileName,
OutputRefFilePath = outputRefFilePath,
PdbPath = pdbPath,
EmitPdb = emitPdb && !metadataOnly,
EmitPdb = emitPdb && !metadataOnly, // silently ignore emitPdb when metadataOnly is set
SourceLink = sourceLink,
OutputDirectory = outputDirectory,
DocumentationPath = documentationPath,
Expand Down
42 changes: 35 additions & 7 deletions src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public abstract class PublicClass
"System.Diagnostics.DebuggableAttribute" });

var peImage = comp.EmitToArray(emitRefOnly);
MetadataReaderUtils.VerifyMethodBodies(peImage, expectEmptyOrThrowNull: true);
MetadataReaderUtils.AssertEmptyOrThrowNull(peImage);
}
[Fact]
public void EmitMetadataOnly_DisallowPdbs()
Expand All @@ -364,7 +364,7 @@ public void EmitMetadataOnly_DisallowMetadataPeStream()
using (var output = new MemoryStream())
using (var metadataPeOutput = new MemoryStream())
{
Assert.Throws<ArgumentException>(() => comp.Emit(output, metadataPeStream: metadataPeOutput,
Assert.Throws<ArgumentException>(() => comp.Emit(output, metadataPEStream: metadataPeOutput,
options: EmitOptions.Default.WithEmitMetadataOnly(true)));
}
}
Expand All @@ -378,7 +378,7 @@ public void EmitMetadata_DisallowOutputtingNetModule()
using (var output = new MemoryStream())
using (var metadataPeOutput = new MemoryStream())
{
Assert.Throws<ArgumentException>(() => comp.Emit(output, metadataPeStream: metadataPeOutput,
Assert.Throws<ArgumentException>(() => comp.Emit(output, metadataPEStream: metadataPeOutput,
options: EmitOptions.Default));
}
}
Expand All @@ -396,6 +396,34 @@ public void EmitMetadataOnly_DisallowOutputtingNetModule()
}
}

[Fact]
public void EmitMetadata_DisallowEmbedding()
{
CSharpCompilation comp = CreateCompilation("", references: new[] { MscorlibRef },
options: TestOptions.DebugDll);

using (var output = new MemoryStream())
using (var metadataOutput = new MemoryStream())
{
Assert.Throws<ArgumentException>(() => comp.Emit(output, metadataPEStream: metadataOutput,
options: EmitOptions.Default.WithDebugInformationFormat(DebugInformationFormat.Embedded)));
}
}

[Fact]
public void EmitMetadataOnly_DisallowEmbedding()
{
CSharpCompilation comp = CreateCompilation("", references: new[] { MscorlibRef },
options: TestOptions.DebugDll);

using (var output = new MemoryStream())
{
Assert.Throws<ArgumentException>(() => comp.Emit(output,
options: EmitOptions.Default.WithEmitMetadataOnly(true)
.WithDebugInformationFormat(DebugInformationFormat.Embedded)));
}
}

[Fact]
public void EmitMetadata()
{
Expand All @@ -412,17 +440,17 @@ public abstract class PublicClass
using (var pdbOutput = new MemoryStream())
using (var metadataOutput = new MemoryStream())
{
var result = comp.Emit(output, pdbOutput, metadataPeStream: metadataOutput);
var result = comp.Emit(output, pdbOutput, metadataPEStream: metadataOutput);
Assert.True(result.Success);
Assert.NotEqual(0, output.Position);
Assert.NotEqual(0, pdbOutput.Position);
Assert.NotEqual(0, metadataOutput.Position);
MetadataReaderUtils.VerifyMethodBodies(ImmutableArray.CreateRange(output.GetBuffer()), expectEmptyOrThrowNull: false);
MetadataReaderUtils.VerifyMethodBodies(ImmutableArray.CreateRange(metadataOutput.GetBuffer()), expectEmptyOrThrowNull: true);
MetadataReaderUtils.AssertNotThrowNull(ImmutableArray.CreateRange(output.GetBuffer()));
MetadataReaderUtils.AssertEmptyOrThrowNull(ImmutableArray.CreateRange(metadataOutput.GetBuffer()));
}

var peImage = comp.EmitToArray();
MetadataReaderUtils.VerifyMethodBodies(peImage, expectEmptyOrThrowNull: false);
MetadataReaderUtils.AssertNotThrowNull(peImage);
}

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public void SymWriterErrors()
{
var result = compilation.Emit(
peStream: peStream,
metadataPeStream: null,
metadataPEStream: null,
pdbStream: pdbStream,
xmlDocumentationStream: null,
cancellationToken: default(CancellationToken),
Expand Down Expand Up @@ -167,7 +167,7 @@ public void SymWriterErrors2()
{
var result = compilation.Emit(
peStream: peStream,
metadataPeStream: null,
metadataPEStream: null,
pdbStream: pdbStream,
xmlDocumentationStream: null,
cancellationToken: default(CancellationToken),
Expand Down Expand Up @@ -201,7 +201,7 @@ public void SymWriterErrors3()
{
var result = compilation.Emit(
peStream: peStream,
metadataPeStream: null,
metadataPEStream: null,
pdbStream: pdbStream,
xmlDocumentationStream: null,
cancellationToken: default(CancellationToken),
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs
View file

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/Core/Portable/CodeAnalysisResources.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,9 @@
<data name="MetadataPeStreamUnexpectedWhenEmittingMetadataOnly" xml:space="preserve">
<value>Metadata PE stream should not be given when emitting metadata only.</value>
</data>
<data name="EmbeddingPdbUnexpectedWhenEmittingMetadata" xml:space="preserve">
<value>Embedding PDB is not allowed when emitting metadata.</value>
</data>
<data name="CannotTargetNetModuleWhenEmittingRefAssembly" xml:space="preserve">
<value>Cannot target net module when emitting ref assembly.</value>
</data>
Expand Down
83 changes: 69 additions & 14 deletions src/Compilers/Core/Portable/Compilation/Compilation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1987,15 +1987,15 @@ public EmitResult Emit(
debugEntryPoint,
sourceLinkStream,
embeddedTexts,
metadataPeStream: null,
metadataPEStream: null,
cancellationToken: cancellationToken);
}

/// <summary>
/// Emit the IL for the compiled source code into the specified stream.
/// </summary>
/// <param name="peStream">Stream to which the compilation will be written.</param>
/// <param name="metadataPeStream">Stream to which the metadata-only output will be written.</param>
/// <param name="metadataPEStream">Stream to which the metadata-only output will be written.</param>
/// <param name="pdbStream">Stream to which the compilation's debug info will be written. Null to forego PDB generation.</param>
/// <param name="xmlDocumentationStream">Stream to which the compilation's XML documentation will be written. Null to forego XML generation.</param>
/// <param name="win32Resources">Stream from which the compilation's Win32 resources will be read (in RES format).
Expand Down Expand Up @@ -2036,7 +2036,7 @@ public EmitResult Emit(
IMethodSymbol debugEntryPoint = null,
Stream sourceLinkStream = null,
IEnumerable<EmbeddedText> embeddedTexts = null,
Stream metadataPeStream = null,
Stream metadataPEStream = null,
CancellationToken cancellationToken = default(CancellationToken))
{
if (peStream == null)
Expand Down Expand Up @@ -2067,16 +2067,22 @@ public EmitResult Emit(
}
}

if (metadataPeStream != null && options?.EmitMetadataOnly == true)
if (metadataPEStream != null && options?.EmitMetadataOnly == true)
{
throw new ArgumentException(CodeAnalysisResources.MetadataPeStreamUnexpectedWhenEmittingMetadataOnly, nameof(metadataPeStream));
throw new ArgumentException(CodeAnalysisResources.MetadataPeStreamUnexpectedWhenEmittingMetadataOnly, nameof(metadataPEStream));
}

if (options?.DebugInformationFormat == DebugInformationFormat.Embedded &&
(metadataPEStream != null || options?.EmitMetadataOnly == true))
{
throw new ArgumentException(CodeAnalysisResources.EmbeddingPdbUnexpectedWhenEmittingMetadata, nameof(metadataPEStream));
}

if (this.Options.OutputKind == OutputKind.NetModule)
{
if (metadataPeStream != null)
if (metadataPEStream != null)
{
throw new ArgumentException(CodeAnalysisResources.CannotTargetNetModuleWhenEmittingRefAssembly, nameof(metadataPeStream));
throw new ArgumentException(CodeAnalysisResources.CannotTargetNetModuleWhenEmittingRefAssembly, nameof(metadataPEStream));
}
else if (options?.EmitMetadataOnly == true)
{
Expand Down Expand Up @@ -2119,7 +2125,7 @@ public EmitResult Emit(

return Emit(
peStream,
metadataPeStream,
metadataPEStream,
pdbStream,
xmlDocumentationStream,
win32Resources,
Expand All @@ -2138,7 +2144,7 @@ public EmitResult Emit(
/// </summary>
internal EmitResult Emit(
Stream peStream,
Stream metadataPeStream,
Stream metadataPEStream,
Stream pdbStream,
Stream xmlDocumentationStream,
Stream win32Resources,
Expand Down Expand Up @@ -2206,7 +2212,7 @@ internal EmitResult Emit(
success = SerializeToPeStream(
moduleBeingBuilt,
new SimpleEmitStreamProvider(peStream),
(metadataPeStream != null) ? new SimpleEmitStreamProvider(metadataPeStream) : null,
(metadataPEStream != null) ? new SimpleEmitStreamProvider(metadataPEStream) : null,
(pdbStream != null) ? new SimpleEmitStreamProvider(pdbStream) : null,
testData?.SymWriterFactory,
diagnostics,
Expand Down Expand Up @@ -2373,7 +2379,7 @@ internal CommonPEModuleBuilder CheckOptionsAndCreateModuleBuilder(
internal bool SerializeToPeStream(
CommonPEModuleBuilder moduleBeingBuilt,
EmitStreamProvider peStreamProvider,
EmitStreamProvider metadataPeStreamProvider,
EmitStreamProvider metadataPEStreamProvider,
EmitStreamProvider pdbStreamProvider,
Func<object> testSymWriterFactory,
DiagnosticBag diagnostics,
Expand Down Expand Up @@ -2487,7 +2493,7 @@ internal bool SerializeToPeStream(
};

Func<Stream> getRefPeStream;
if (metadataPeStreamProvider != null)
if (metadataPEStreamProvider != null)
{
getRefPeStream = () =>
{
Expand All @@ -2496,7 +2502,7 @@ internal bool SerializeToPeStream(
return null;
}

refPeStream = metadataPeStreamProvider.GetOrCreateStream(metadataDiagnostics);
refPeStream = metadataPEStreamProvider.GetOrCreateStream(metadataDiagnostics);
Debug.Assert(refPeStream != null || metadataDiagnostics.HasAnyErrors());
return refPeStream;
};
Expand All @@ -2508,7 +2514,7 @@ internal bool SerializeToPeStream(

try
{
if (Cci.PeWriter.WritePeToStream(
if (SerializePeToStream(
new EmitContext(moduleBeingBuilt, null, metadataDiagnostics),
this.MessageProvider,
getPeStream,
Expand Down Expand Up @@ -2591,6 +2597,55 @@ internal bool SerializeToPeStream(
return true;
}

internal static bool SerializePeToStream(
EmitContext context,
CommonMessageProvider messageProvider,
Func<Stream> getPeStream,
Func<Stream> getMetadataPeStreamOpt,
Func<Stream> getPortablePdbStreamOpt,
Cci.PdbWriter nativePdbWriterOpt,
string pdbPathOpt,
bool metadataOnly,
bool isDeterministic,
CancellationToken cancellationToken)
{
if (!Cci.PeWriter.WritePeToStream(
context,
messageProvider,
getPeStream,
getPortablePdbStreamOpt,
nativePdbWriterOpt,
pdbPathOpt,
metadataOnly,
isDeterministic,
cancellationToken))
{
return false;
}

// produce the secondary output (ref assembly) if needed
if (getMetadataPeStreamOpt != null)
{
Debug.Assert(!metadataOnly);

if (!Cci.PeWriter.WritePeToStream(
context,
messageProvider,
getMetadataPeStreamOpt,
getPortablePdbStreamOpt: null,
nativePdbWriterOpt: null,
pdbPathOpt: null,
metadataOnly: true,
isDeterministic: isDeterministic,
cancellationToken: cancellationToken))
{
return false;
}
}

return true;
}

internal EmitBaseline SerializeToDeltaStreams(
CommonPEModuleBuilder moduleBeingBuilt,
EmitBaseline baseline,
Expand Down
Loading

0 comments on commit b360fd0

Please sign in to comment.