Skip to content

Conversation

@aschackmull
Copy link
Contributor

This updates the class and predicate names on these two shared libraries so they align with the naming laid out in the shared SSA library.

Copilot AI review requested due to automatic review settings October 23, 2025 08:59
@aschackmull aschackmull requested a review from a team as a code owner October 23, 2025 08:59
@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Oct 23, 2025
@aschackmull aschackmull requested review from a team as code owners October 23, 2025 08:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aligns the naming conventions for SSA-related classes and predicates in the Guards and ControlFlowReachability libraries with the shared SSA library's standard naming scheme.

Key changes include:

  • Renaming SsaWriteDefinition to SsaExplicitWrite and its getDefinition() method to getValue()
  • Renaming SsaPhiNode to SsaPhiDefinition
  • Replacing parameterDefinition predicate with SsaParameterInit class
  • Renaming SsaUncertainDefinition to SsaUncertainWrite

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
shared/controlflow/codeql/controlflow/Guards.qll Updates shared Guards library interface definitions and all references to use new SSA naming conventions
shared/controlflow/codeql/controlflow/ControlFlowReachability.qll Updates shared ControlFlowReachability library interface definitions and all references to use new SSA naming conventions
java/ql/lib/semmle/code/java/controlflow/Guards.qll Updates Java Guards implementation to conform to new SSA interface naming for both v1 and v2 logic inputs
java/ql/lib/semmle/code/java/controlflow/ControlFlowReachability.qll Updates Java ControlFlowReachability implementation to use new SSA class names
csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll Updates C# Guards implementation to conform to new SSA interface naming
csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowReachability.qll Updates C# ControlFlowReachability implementation to use new SSA class names
cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll Updates C++ IRGuards implementation to conform to new SSA interface naming

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +393 to +396
class SsaParameterInit extends SsaDefinition {
SsaParameterInit() { this.isParameterDefinition(_) }

GuardsInput::Parameter getParameter() { this.isParameterDefinition(result) }
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be slightly more than a renaming. Did you check that this doesn't affect join-orders?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It replaces one binary predicate with another - it shouldn't make a difference whether it's a member predicate. But since you asked, I just did a DIL diff of a suitable query and verified that the generated DIL is equivalent.

@aschackmull aschackmull merged commit 6f72234 into github:main Oct 23, 2025
51 checks passed
@aschackmull aschackmull deleted the shared/align-ssa-interface branch October 23, 2025 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C# C++ Java no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants