IOperation Test Porting Pt 3#51206
Conversation
… if they do not have one. (cherry picked from commit 4af2613)
src/Compilers/Test/Core/Compilation/ControlFlowGraphVerifier.cs
Outdated
Show resolved
Hide resolved
|
@dotnet/roslyn-compiler @AlekseyTs for review. |
src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs
Outdated
Show resolved
Hide resolved
|
Done with review pass (commit 9) #Closed |
* Simplify named argument binding by making all named args specifically BoundAssignmentOperators. They already were, this carries the knowledge through the rest of the code. * Add tests for invalid attribute binding scenarios. * Only bind constructor params for error recovery in bad cases. * Use the correct binder when binding for error recover to avoid runaway binding.
|
@AlekseyTs addressed feedback. I've split the resolutions into separate commits as they followup on different iteration comments. In reply to: 780630534 [](ancestors = 780630534) |
|
@AlekseyTs @dotnet/roslyn-compiler for review. |
|
Done with review pass (commit 11) #Closed |
…oser to overload resolution, remove unneeded frees.
|
@AlekseyTs addressed feedback. In reply to: 784305142 [](ancestors = 784305142) |
AlekseyTs
left a comment
There was a problem hiding this comment.
LGTM (commit 13), assuming CI is passing.
|
@dotnet/roslyn-compiler for a second review please. |
| { | ||
| if ((object)_resolved == null) | ||
| { | ||
| Debug.Assert(_underlying.IsSafeToResolve()); |
There was a problem hiding this comment.
Why remove this assertion? #Closed
Please update the comment (the result now also holds pooled builders which the caller is responsible for freeing) #Closed Refers to: src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs:305 in 76e0ccd. [](commit_id = 76e0ccd, deletion_comment = False) |
| else | ||
| { | ||
| boundConstructorArguments = analyzedArguments.ConstructorArguments.Arguments.SelectAsArray( | ||
| (arg, attributeArgumentBinder) => attributeArgumentBinder.BindToTypeForErrorRecovery(arg), |
There was a problem hiding this comment.
(arg, attributeArgumentBinder) [](start = 20, length = 30)
static? #Closed
jcouv
left a comment
There was a problem hiding this comment.
Done with review pass (iteration 13)
This ports the last of the commits from #49534 and addresses feedback on them.
The only remaining test hook failures in master were introduced by the UsedAssemblyReferences branch and Aleksey is taking a look at those. If that isn't fixed by the time this is merged, I'll turn those tests off for the test hook when I enable the test hook in CI.After investigation, these failures are invalid, and the assert is removed by the last commit in this PR.As in my previous porting PRs, ported commits are prefixed with Port:, and fixup commits are prefixed with Fixup:. There are 4 new, previously unreviewed commits at the end that are not prefixed. See the commit descriptions for what each one does.