Skip to content

C#: Fix CFG for assertions with multiple assertion arguments#4568

Merged
hvitved merged 2 commits intogithub:mainfrom
hvitved:csharp/cfg/multi-asserts
Oct 30, 2020
Merged

C#: Fix CFG for assertions with multiple assertion arguments#4568
hvitved merged 2 commits intogithub:mainfrom
hvitved:csharp/cfg/multi-asserts

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Oct 28, 2020

Follow-up on #4484.

The control-flow graph for M13 below

private void AssertTrueFalse(
       [System.Diagnostics.CodeAnalysis.DoesNotReturnIf(false)] bool condition1,
       [System.Diagnostics.CodeAnalysis.DoesNotReturnIf(true)] bool condition2,
       bool nonCondition)
{
}

void M13(bool b1, bool b2, bool b3)
{
    AssertTrueFalse(b1, b2, b3);
    return;
}

now becomes

Screenshot 2020-10-28 at 19 58 22

(Note that the graph visualizer is, for some reason, not able to show both the true and false edges out of [assertion failure] access to parameter b2 (highlighted in red), possibly because the two edges have the same target.)

CSharp-Differences job here.

@hvitved hvitved force-pushed the csharp/cfg/multi-asserts branch 2 times, most recently from a20500b to 39e42e3 Compare October 29, 2020 09:37
@hvitved hvitved marked this pull request as ready for review October 29, 2020 09:44
@hvitved hvitved requested a review from a team as a code owner October 29, 2020 09:44
tamasvajk
tamasvajk previously approved these changes Oct 29, 2020
Copy link
Contributor

@tamasvajk tamasvajk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hvitved
Copy link
Contributor Author

hvitved commented Oct 30, 2020

CSharp-Changes looks good, so merging.

@hvitved hvitved merged commit 91d7294 into github:main Oct 30, 2020
@hvitved hvitved deleted the csharp/cfg/multi-asserts branch October 30, 2020 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants