More tests for 'with(...)' element scenarios. #80831
Conversation
|
I'm not sure how to finish reviewing this PR. It seems to have irrelevant commits directly added to the commit list, not as a simple merge. For comparison, when I did a merge from 
|
I assume because @CyrusNajmabadi merged main into this branch, and it isn't in the feature branch currently. |
…-arguments' into withError2
|
PR should be good again. |
|
Ok. I think review feedback accounted for. |
|
Would like to move further tests to other followups. |
|
|
||
| // PROTOTYPE: Spec says we should only allow arglist if trivial. Circle back on this and see if | ||
| // this just falls out with the 'allowArgList: true' below. If so, let LDM know it was easy and | ||
| // allow it. If it requires substantial work beyond this, disallow it for this feature. |
There was a problem hiding this comment.
resolved by email.
| <Field Name="Type" Type="TypeSymbol?" Override="true" Null="always"/> | ||
|
|
||
| <!-- With-arguments support for collection expression with(...) syntax. | ||
| PROTOTYPE: Determine if this is how we want to represent this data in this expression. --> |
There was a problem hiding this comment.
resolved by meeting today. #Closed
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Constructor.cs
Show resolved
Hide resolved
| var root = semanticModel.SyntaxTree.GetRoot(); | ||
|
|
||
| var (graph, symbol) = ControlFlowGraphVerifier.GetControlFlowGraph(root.DescendantNodes().OfType<BlockSyntax>().ToArray()[1], semanticModel); | ||
| ControlFlowGraphVerifier.VerifyGraph(compilation, """ |
There was a problem hiding this comment.
Uh... what happened to the ternary? This seems broken, I'd expect to see that extracted out and feeding into the collection creation as a flow capture. It also appears that the operation tree writer is not including the constructor in the output of a ICollectionExpressionOperation.
There was a problem hiding this comment.
Looks like ControlFlowGraph depends on IOp, which likely needs to be updated to include the creation expr. i'd really like to not do that in this PR if that's ok.
CyrusNajmabadi
force-pushed
the
withError2
branch
from
6c43cb7 to
b9fc660
Compare
333fred
left a comment
333fred
left a comment
There was a problem hiding this comment.
Do we have a prototype comment tracking the IOperation work yet?
|
Not that i know of. |
Please add one. |
Sure. Added. |
CyrusNajmabadi
merged commit 329e403
into
dotnet:features/collection-expression-arguments
3 participants
Also adds some support fo GetSymbolInfo on a with(...) element.
Relates to test plan #80613