Respond to PR feedback

This change takes the following steps:

- Changes EmitStreamProvider to wrap a single Stream instead of a group
of Stream.
- Changes the emit behavior to consider Analyzer errors before
attempting to write the PE / PDB Stream to disk.

src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs

@@ -3875,6 +3875,7 @@ public void SymWriterErrors()
                     win32Resources: null,
                     manifestResources: null,
                     options: null,
+                    getHostDiagnostics: null,
                     testData: new CompilationTestData() { SymWriterFactory = () => new MockSymUnmanagedWriter() });
 
                 result.Diagnostics.Verify(

src/Compilers/Core/Desktop/CommandLine/CommonCompiler.CompilerEmitStreamProvider.cs

@@ -10,7 +10,7 @@ internal abstract partial class CommonCompiler
     {
         /// <summary>
         /// This implementation of <see cref="Compilation.EmitStreamProvider"/> will delay the creation
-        /// of the PE + PDB file until the compiler determines the compilation has succeeded.  This prevents
+        /// of the PE / PDB file until the compiler determines the compilation has succeeded.  This prevents
         /// the compiler from deleting output from the previous compilation when a new compilation 
         /// fails.
         /// </summary>
@@ -18,52 +18,30 @@ private sealed class CompilerEmitStreamProvider : Compilation.EmitStreamProvider
         {
             private readonly CommonCompiler _compiler;
             private readonly TouchedFileLogger _touchedFileLogger;
-            private readonly string _peFilePath;
-            private readonly string _pdbFilePath;
-            private Stream _peStream;
-            private Stream _pdbStream;
+            private readonly string _filePath;
+            private Stream _stream;
 
-            internal CompilerEmitStreamProvider(CommonCompiler compiler, TouchedFileLogger touchedFileLogger, string peFilePath, string pdbFilePath)
+            internal CompilerEmitStreamProvider(CommonCompiler compiler, TouchedFileLogger touchedFileLogger, string filePath)
             {
                 _compiler = compiler;
                 _touchedFileLogger = touchedFileLogger;
-                _peFilePath = peFilePath;
-                _pdbFilePath = pdbFilePath;
+                _filePath = filePath;
             }
 
             public void Dispose()
             {
-                _peStream?.Dispose();
-                _peStream = null;
-                _pdbStream?.Dispose();
-                _pdbStream = null;
+                _stream?.Dispose();
+                _stream = null;
             }
 
-            public override bool HasPdbStream
+            public override Stream GetStream(DiagnosticBag diagnostics)
             {
-                get { return _pdbFilePath != null; }
-            }
-
-            public override Stream GetPeStream(DiagnosticBag diagnostics)
-            {
-                if (_peStream == null)
+                if (_stream == null)
                 {
-                    _peStream = OpenFile(_peFilePath, diagnostics);
+                    _stream = OpenFile(_filePath, diagnostics);
                 }
 
-                return _peStream;
-            }
-
-            public override Stream GetPdbStream(DiagnosticBag diagnostics)
-            {
-                Debug.Assert(HasPdbStream);
-
-                if (_pdbStream == null)
-                {
-                    _pdbStream = OpenFile(_pdbFilePath, diagnostics);
-                }
-
-                return _pdbStream;
+                return _stream;
             }
 
             private Stream OpenFile(string filePath, DiagnosticBag diagnostics)
@@ -87,35 +65,5 @@ private Stream OpenFile(string filePath, DiagnosticBag diagnostics)
                 }
             }
         }
-
-        private sealed class SimpleEmitStreamProvider : Compilation.EmitStreamProvider
-        {
-            private readonly Stream _peStream;
-            private readonly Stream _pdbStream;
-
-            internal SimpleEmitStreamProvider(Stream peStream, Stream pdbStream = null)
-            {
-                Debug.Assert(peStream.CanWrite);
-                Debug.Assert(pdbStream == null || pdbStream.CanWrite);
-                _peStream = peStream;
-                _pdbStream = pdbStream;
-            }
-
-            public override bool HasPdbStream
-            {
-                get { return _pdbStream != null; }
-            }
-
-            public override Stream GetPeStream(DiagnosticBag diagnostics)
-            {
-                return _peStream;
-            }
-
-            public override Stream GetPdbStream(DiagnosticBag diagnostics)
-            {
-                Debug.Assert(HasPdbStream);
-                return _pdbStream;
-            }
-        }
     }
 }
\ No newline at end of file

src/Compilers/Core/Desktop/CommandLine/CommonCompiler.cs

@@ -330,16 +330,19 @@ private int RunCore(TextWriter consoleOutput, CancellationToken cancellationToke
 
             var analyzerOptions = new AnalyzerOptions(ImmutableArray.Create<AdditionalText, AdditionalTextFile>(additionalTextFiles));
 
-            AnalyzerDriver analyzerDriver = null;
-            AnalyzerManager analyzerManager = null;
-            ConcurrentSet<Diagnostic> analyzerExceptionDiagnostics = null;
+            Func<ImmutableArray<Diagnostic>> getAnalyzerDiagnostics = null;
             if (!analyzers.IsDefaultOrEmpty)
             {
-                analyzerManager = new AnalyzerManager();
-                analyzerExceptionDiagnostics = new ConcurrentSet<Diagnostic>();
+                var analyzerManager = new AnalyzerManager();
+                var analyzerExceptionDiagnostics = new ConcurrentSet<Diagnostic>();
                 Action<Diagnostic> addExceptionDiagnostic = diagnostic => analyzerExceptionDiagnostics.Add(diagnostic);
 
-                analyzerDriver = AnalyzerDriver.Create(compilation, analyzers, analyzerOptions, analyzerManager, addExceptionDiagnostic, out compilation, cancellationToken);
+                var analyzerDriver = AnalyzerDriver.Create(compilation, analyzers, analyzerOptions, analyzerManager, addExceptionDiagnostic, out compilation, cancellationToken);
+                getAnalyzerDiagnostics = () =>
+                    {
+                        var analyzerDiagnostics = analyzerDriver.GetDiagnosticsAsync().Result;
+                        return analyzerDiagnostics.AddRange(analyzerExceptionDiagnostics);
+                    };
             }
 
             // Print the diagnostics produced during the parsing stage and exit if there were any errors.
@@ -401,9 +404,10 @@ private int RunCore(TextWriter consoleOutput, CancellationToken cancellationToke
                     WithOutputNameOverride(outputName).
                     WithPdbFilePath(finalPdbFilePath);
 
-                using (var emitStreamProvider = new CompilerEmitStreamProvider(this, touchedFilesLogger, finalOutputPath, Arguments.EmitPdb ? finalPdbFilePath : null))
+                using (var peStreamProvider = new CompilerEmitStreamProvider(this, touchedFilesLogger, finalOutputPath))
+                using (var pdbStreamProvider = Arguments.EmitPdb ? new CompilerEmitStreamProvider(this, touchedFilesLogger, finalPdbFilePath) : null)
                 {
-                    emitResult = compilation.Emit(emitStreamProvider, xml, win32Res, Arguments.ManifestResources, emitOptions, cancellationToken);
+                    emitResult = compilation.Emit(peStreamProvider, pdbStreamProvider, xml, win32Res, Arguments.ManifestResources, emitOptions, getAnalyzerDiagnostics, cancellationToken);
                 }
             }
 
@@ -416,19 +420,6 @@ private int RunCore(TextWriter consoleOutput, CancellationToken cancellationToke
 
             cancellationToken.ThrowIfCancellationRequested();
 
-            if (analyzerDriver != null)
-            {
-                var analyzerDiagnostics = analyzerDriver.GetDiagnosticsAsync().Result;
-                var allAnalyzerDiagnostics = analyzerDiagnostics.AddRange(analyzerExceptionDiagnostics);
-
-                if (PrintErrors(allAnalyzerDiagnostics, consoleOutput))
-                {
-                    return Failed;
-                }
-
-                cancellationToken.ThrowIfCancellationRequested();
-            }
-
             bool errorsReadingAdditionalFiles = false;
             foreach (var additionalFile in additionalTextFiles)
             {
@@ -749,7 +740,7 @@ internal int RunInteractive(TextWriter consoleOutput)
             byte[] compiledAssembly;
             using (MemoryStream output = new MemoryStream())
             {
-                EmitResult emitResult = compilation.Emit(new SimpleEmitStreamProvider(output));
+                EmitResult emitResult = compilation.Emit(output);
                 if (PrintErrors(emitResult.Diagnostics, consoleOutput))
                 {
                     return Failed;

src/Compilers/Core/Portable/Compilation.EmitStreamProvider.cs

@@ -21,42 +21,26 @@ public abstract partial class Compilation
         /// </summary>
         internal abstract class EmitStreamProvider
         {
-            public abstract bool HasPdbStream
-            {
-                get;
-            }
-
-            public abstract Stream GetPeStream(DiagnosticBag diagnostics);
-
-            public abstract Stream GetPdbStream(DiagnosticBag diagnostics);
+            /// <summary>
+            /// This method will be called once during Emit at the time the Compilation needs the
+            /// associated <see cref="Stream"/> for writing.  It will not be called in the case of
+            /// user errors in code.
+            /// </summary>
+            public abstract Stream GetStream(DiagnosticBag diagnostics);
         }
 
         private sealed class SimpleEmitStreamProvider : EmitStreamProvider
         {
-            private readonly Stream _peStream;
-            private readonly Stream _pdbStream;
-
-            internal SimpleEmitStreamProvider(Stream peStream, Stream pdbStream = null)
-            {
-                Debug.Assert(peStream.CanWrite);
-                Debug.Assert(pdbStream == null || pdbStream.CanWrite);
-                _peStream = peStream;
-                _pdbStream = pdbStream;
-            }
-
-            public override bool HasPdbStream
-            {
-                get { return _pdbStream != null; }
-            }
+            private readonly Stream _stream;
 
-            public override Stream GetPeStream(DiagnosticBag diagnostics)
+            internal SimpleEmitStreamProvider(Stream stream)
             {
-                return _peStream;
+                _stream = stream;
             }
 
-            public override Stream GetPdbStream(DiagnosticBag diagnostics)
+            public override Stream GetStream(DiagnosticBag diagnostics)
             {
-                return _pdbStream;
+                return _stream;
             }
         }
     }

src/Compilers/Core/Portable/Compilation/Compilation.cs

@@ -1354,39 +1354,48 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
             }
 
             return Emit(
-                new SimpleEmitStreamProvider(peStream, pdbStream),
+                peStream,
+                pdbStream,
                 xmlDocumentationStream,
                 win32Resources,
                 manifestResources,
                 options,
-                cancellationToken);
+                testData: null,
+                getHostDiagnostics: null,
+                cancellationToken: cancellationToken);
         }
 
         /// <summary>
         /// Emit the IL for the compiled source code into the specified stream.
         /// </summary>
-        /// <param name="emitStreamProvider">Provides stream the compiler will write to.</param>
+        /// <param name="peStreamProvider">Provides the PE stream the compiler will write to.</param>
+        /// <param name="pdbStreamProvider">Provides the PDB stream the compiler will write to.</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).  
         /// Null to indicate that there are none. The RES format begins with a null resource entry.</param>
         /// <param name="manifestResources">List of the compilation's managed resources.  Null to indicate that there are none.</param>
         /// <param name="options">Emit options.</param>
+        /// <param name="getHostDiagnostics">Returns any extra diagnostics produced by the host of the compiler.</param>
         /// <param name="cancellationToken">To cancel the emit process.</param>
         internal EmitResult Emit(
-            EmitStreamProvider emitStreamProvider,
+            EmitStreamProvider peStreamProvider,
+            EmitStreamProvider pdbStreamProvider,
             Stream xmlDocumentationStream = null,
             Stream win32Resources = null,
             IEnumerable<ResourceDescription> manifestResources = null,
             EmitOptions options = null,
+            Func<ImmutableArray<Diagnostic>> getHostDiagnostics = null,
             CancellationToken cancellationToken = default(CancellationToken))
         {
             return Emit(
-                emitStreamProvider,
+                peStreamProvider,
+                pdbStreamProvider,
                 xmlDocumentationStream,
                 win32Resources,
                 manifestResources,
                 options,
                 testData: null,
+                getHostDiagnostics: getHostDiagnostics,
                 cancellationToken: cancellationToken);
         }
 
@@ -1486,15 +1495,18 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
             IEnumerable<ResourceDescription> manifestResources,
             EmitOptions options,
             CompilationTestData testData,
+            Func<ImmutableArray<Diagnostic>> getHostDiagnostics,
             CancellationToken cancellationToken)
         {
             return Emit(
-                new SimpleEmitStreamProvider(peStream, pdbStream),
+                new SimpleEmitStreamProvider(peStream),
+                pdbStream != null ? new SimpleEmitStreamProvider(pdbStream) : null,
                 xmlDocumentationStream,
                 win32Resources,
                 manifestResources,
                 options,
                 testData,
+                getHostDiagnostics,
                 cancellationToken);
         }
 
@@ -1504,15 +1516,17 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
         /// </summary>
         /// <returns>True if emit succeeded.</returns>
         internal EmitResult Emit(
-            EmitStreamProvider emitStreamProvider,
+            EmitStreamProvider peStreamProvider,
+            EmitStreamProvider pdbStreamProvider,
             Stream xmlDocumentationStream,
             Stream win32Resources,
             IEnumerable<ResourceDescription> manifestResources,
             EmitOptions options,
             CompilationTestData testData,
+            Func<ImmutableArray<Diagnostic>> getHostDiagnostics,
             CancellationToken cancellationToken)
         {
-            Debug.Assert(emitStreamProvider != null);
+            Debug.Assert(peStreamProvider != null);
 
             DiagnosticBag diagnostics = new DiagnosticBag();
             if (options != null)
@@ -1558,7 +1572,7 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
                 moduleBeingBuilt,
                 win32Resources,
                 xmlDocumentationStream,
-                generateDebugInfo: emitStreamProvider.HasPdbStream,
+                generateDebugInfo: pdbStreamProvider != null,
                 diagnostics: diagnostics,
                 filterOpt: null,
                 cancellationToken: cancellationToken))
@@ -1566,9 +1580,19 @@ internal void EnsureAnonymousTypeTemplates(CancellationToken cancellationToken)
                 return ToEmitResultAndFree(diagnostics, success: false);
             }
 
+            var hostDiagnostics = getHostDiagnostics != null
+                ? getHostDiagnostics()
+                : ImmutableArray<Diagnostic>.Empty;
+            diagnostics.AddRange(hostDiagnostics);
+            if (hostDiagnostics.Any(x => x.Severity == DiagnosticSeverity.Error))
+            {
+                return ToEmitResultAndFree(diagnostics, success: false);
+            }
+
             bool success = SerializeToPeStream(
                 moduleBeingBuilt,
-                emitStreamProvider,
+                peStreamProvider,
+                pdbStreamProvider,
                 options.PdbFilePath,
                 (testData != null) ? testData.SymWriterFactory : null,
                 diagnostics,
@@ -1585,14 +1609,15 @@ private EmitResult ToEmitResultAndFree(DiagnosticBag diagnostics, bool success)
 
         internal bool SerializeToPeStream(
             CommonPEModuleBuilder moduleBeingBuilt,
-            EmitStreamProvider emitStreamProvider,
+            EmitStreamProvider peStreamProvider,
+            EmitStreamProvider pdbStreamProvider,
             string pdbFileName,
             Func<object> testSymWriterFactory,
             DiagnosticBag diagnostics,
             bool metadataOnly,
             CancellationToken cancellationToken)
         {
-            Stream peStream = emitStreamProvider.GetPeStream(diagnostics);
+            Stream peStream = peStreamProvider.GetStream(diagnostics);
             if (peStream == null)
             {
                 Debug.Assert(diagnostics.HasAnyErrors());
@@ -1601,9 +1626,9 @@ private EmitResult ToEmitResultAndFree(DiagnosticBag diagnostics, bool success)
             Debug.Assert(peStream.CanWrite);
 
             Stream pdbStream = null;
-            if (emitStreamProvider.HasPdbStream)
+            if (pdbStreamProvider != null)
             {
-                pdbStream = emitStreamProvider.GetPdbStream(diagnostics);
+                pdbStream = pdbStreamProvider.GetStream(diagnostics);
                 if (pdbStream == null)
                 {
                     Debug.Assert(diagnostics.HasAnyErrors());

src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/ExpressionCompilerTests.cs

@@ -3761,6 +3761,7 @@ static void M()
                         manifestResources: null,
                         options: EmitOptions.Default,
                         testData: testData0,
+                        getHostDiagnostics: null,
                         cancellationToken: default(CancellationToken));
                     exeBytes = exeStream.ToArray();
                     pdbBytes = pdbStream.ToArray();
@@ -3860,6 +3861,7 @@ static void M()
                         manifestResources: null,
                         options: EmitOptions.Default,
                         testData: testData0,
+                        getHostDiagnostics: null,
                         cancellationToken: default(CancellationToken));
                     exeBytes = exeStream.ToArray();
                     pdbBytes = pdbStream.ToArray();

src/ExpressionEvaluator/Core/Test/ExpressionCompiler/ExpressionCompilerTestHelpers.cs

@@ -172,6 +172,7 @@ internal static ImmutableArray<string> GetLocalNames(this ISymUnmanagedReader sy
                         manifestResources: null,
                         options: EmitOptions.Default,
                         testData: null,
+                        getHostDiagnostics: null,
                         cancellationToken: default(CancellationToken));
 
                     if (!result.Success)

src/Test/Utilities/CompilationExtensions.cs

@@ -31,6 +31,7 @@ public static class CompilationExtensions
                 manifestResources: null,
                 options: options,
                 testData: testData,
+                getHostDiagnostics: null,
                 cancellationToken: default(CancellationToken));
 
             Assert.True(emitResult.Success, "Diagnostics:\r\n" + string.Join("\r\n, ", emitResult.Diagnostics.Select(d => d.ToString())));

src/Test/Utilities/HostedRuntimeEnvironment.cs

@@ -222,6 +222,7 @@ private static IEnumerable<ModuleMetadata> EnumerateModules(Metadata metadata)
                         manifestResources: manifestResources,
                         options: EmitOptions.Default,
                         testData: testData,
+                        getHostDiagnostics: null,
                         cancellationToken: default(CancellationToken));
                 }
                 finally