VB: Unify CreateCompilation helpers#25130
Conversation
|
CC @dotnet/roslyn-compiler for review |
NuGet.Config
Outdated
There was a problem hiding this comment.
DO NOT MERGE [](start = 14, length = 12)
This means "do not merge to master" or "do not merge at all"?
There was a problem hiding this comment.
That was a reminder to myself to delete this line. I failed.
There was a problem hiding this comment.
Nit: assemblyName:= probably can be removed.
|
Sounds like you're going to refresh this PR. Ping me afterwards to resume the review. Thanks |
Move the source based CompileAndVerifyFieldMarshal overloads into the language specific layer. This is a step closer to removing the GetCompilationForEmit method.
All of the source based operations are now in the language specific heplers.
|
@jcouv this is ready for review now. |
|
CC @roslyn-compiler for review. The change is actually focused in the following files:
Everything else is just an API change reacting to these changes. |
| Dim assemblyName As String = Nothing | ||
| Dim sourceTrees = ParseSourceXml(source, parseOptions, assemblyName) | ||
| Dim compilation = CreateEmptyCompilation(sourceTrees, allReferences, options, assemblyName) | ||
| Dim compilation = CreateEmptyCompilation(sourceTrees.ToArray(), allReferences, options, assemblyName:=assemblyName) |
There was a problem hiding this comment.
.ToArray() [](start = 60, length = 10)
Is .ToArray() necessary here?
There was a problem hiding this comment.
Yes. There is no way to do an implicit conversion from IEnumerable(Of SyntaxTree) to BasicTestSource as we don't allow widening conversions on interfaces.
In reply to: 192156452 [](ancestors = 192156452)
| Dim defaultRefs = If(useLatestFrameworkReferences, LatestVbReferences, DefaultVbReferences) | ||
| Dim compilation = CreateCompilationWithMscorlib45AndVBRuntime({syntaxTree}, references:=defaultRefs.Append({ValueTupleRef, SystemRuntimeFacadeRef}), options:=If(compilationOptions, TestOptions.ReleaseDll)) | ||
| Dim allReferences As IEnumerable(Of MetadataReference) | ||
| If useLatestFrameworkReferences Then |
There was a problem hiding this comment.
useLatestFrameworkReferences [](start = 11, length = 28)
Consider moving useLatestFrameworkReferences check into .Add() call.
There was a problem hiding this comment.
| Dim defaultRefs = If(useLatestFramework, LatestVbReferences, DefaultVbReferences) | ||
| Dim allReferences = defaultRefs.Concat({ValueTupleRef, SystemRuntimeFacadeRef}) | ||
| Dim allReferences As IEnumerable(Of MetadataReference) = Nothing | ||
| If useLatestFramework Then |
There was a problem hiding this comment.
useLatestFramework [](start = 11, length = 18)
Consider moving into .Add() call.
There was a problem hiding this comment.
|
|
||
| Dim references = {MscorlibRef, SystemRef, MsvbRef} | ||
| Return CreateEmptyCompilation(source, references, options, assemblyName) | ||
| Return CreateEmptyCompilation(source.ToArray(), references, options:=options, assemblyName:=assemblyName) |
There was a problem hiding this comment.
.ToArray() [](start = 44, length = 10)
Is .ToArray() necessary?
Next phase in the VB test helper changes. This unifies our CreateCompilation helpers to match the style of the C# counterparts.
Items specifically excluded from this change:
Both of these items are coming but they are largely refactoring operations. Going to separate that out into a separate PR to help with the reviews.