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 |
jaredpar
added
the
PR For Personal Review Only
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.
jaredpar
added
PR For Personal Review Only
jaredpar
removed
the
PR For Personal Review Only
|
@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?
4 participants
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.