Sitelet https://web.archive.org/web/20260603102431/https://github.com/github/codeql/pull/4959
Skip to content

C#: Split up SSA implementation#4959

Merged
hvitved merged 2 commits into
github:mainfrom
hvitved:csharp/ssa/split
Jan 21, 2021
Merged

C#: Split up SSA implementation#4959
hvitved merged 2 commits into
github:mainfrom
hvitved:csharp/ssa/split

Conversation

@hvitved
Copy link
Copy Markdown
Contributor

@hvitved hvitved commented Jan 14, 2021 •

This PR splits up SSA.qll into four files:

  • SsaImplCommon.qll: This file contains all the language-agnostic details of the SSA implementation. Eventually this file will be shared.
  • SsaImplSpecific.qll: This file contains the C#-specific input to the shared library above. The interface is simple: Define basic blocks, basic block dominance, reads, and writes.
  • SsaImpl.qll: This file contains additional internal C#-specific implementation details.
  • SSA.qll: Contains the exposed SSA interface.

https://jenkins.internal.semmle.com/job/Changes/job/CSharp-Differences/772/

Sadly, this PR results in some slow-down, because the cached CFG stage is reevaluated. I tried to avoid it by adding additional import csharp statements, but without luck. I have documented the problem here, and I guess we just have to wait for an optimizer fix.

@github-actions github-actions Bot added the C# label Jan 14, 2021
@hvitved hvitved marked this pull request as ready for review January 18, 2021 11:49
@hvitved hvitved requested a review from a team as a code owner January 18, 2021 11:49
Copy link
Copy Markdown
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 hvitved merged commit bc41c26 into github:main Jan 21, 2021
@hvitved hvitved deleted the csharp/ssa/split branch January 21, 2021 09:52
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