Skip to content

C#: Unify goto completions#1548

Merged
semmle-qlci merged 3 commits intogithub:masterfrom
hvitved:csharp/cfg/simplify-goto-completions
Aug 12, 2019
Merged

C#: Unify goto completions#1548
semmle-qlci merged 3 commits intogithub:masterfrom
hvitved:csharp/cfg/simplify-goto-completions

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Jul 4, 2019

The last commit fixes a minor performance regression caused by #1486 (profiling report), most notable on mono. Performance report for this PR is here.

@hvitved hvitved force-pushed the csharp/cfg/simplify-goto-completions branch from 651e933 to f56c17f Compare July 5, 2019 05:21
@hvitved hvitved marked this pull request as ready for review August 6, 2019 08:06
@hvitved hvitved requested a review from a team as a code owner August 6, 2019 08:06
@hvitved hvitved requested a review from calumgrant August 6, 2019 08:06
Copy link
Contributor

@calumgrant calumgrant left a comment

Choose a reason for hiding this comment

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

Overall, the QL is clearly a great improvement. One thing I didn't understand is how the results have changed. I'm sure they're fine but it's quite hard to tell from the table of .expected output.

@hvitved
Copy link
Contributor Author

hvitved commented Aug 12, 2019

One thing I didn't understand is how the results have changed.

The reason for this is that the successor of a goto label/case/default statement is now uniformly the targeted labeled statement (LabeledStmt), rather than the nested statement under the label. This was already the case for all gotos, except goto cases.

@semmle-qlci semmle-qlci merged commit e27b373 into github:master Aug 12, 2019
@hvitved hvitved deleted the csharp/cfg/simplify-goto-completions branch August 12, 2019 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants