Skip to content

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Sep 4, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings September 4, 2025 11:05
@github-actions github-actions bot added the Rust Pull requests that update Rust code label Sep 4, 2025
Copy link
Contributor

@Copilot 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 refactors the TraitIsVisible module to use the more efficient doublyBoundedFastTC predicate instead of a custom recursive lookup implementation. The change improves performance by leveraging CodeQL's optimized transitive closure computation for determining trait visibility.

Key Changes

  • Replaced recursive traitLookup predicate with a graph-based approach using doublyBoundedFastTC
  • Introduced a new type system (TNode) to represent the nodes in the visibility graph
  • Simplified the main traitIsVisible predicate logic

Comment on lines +1436 to +1439
exists(Trait t1, ItemNode i2 |
n1 = TTrait(t1) and
n2 = TItemNode(i2) and
t1 = i2.getASuccessor(_, _)
Copy link
Preview

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

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

The getASuccessor call is missing the required name parameter. Based on the context, this should be t1 = i2.getASuccessor(_, _) where the first underscore represents the name parameter.

Suggested change
exists(Trait t1, ItemNode i2 |
n1 = TTrait(t1) and
n2 = TItemNode(i2) and
t1 = i2.getASuccessor(_, _)
exists(Trait t1, ItemNode i2, string name |
n1 = TTrait(t1) and
n2 = TItemNode(i2) and
t1 = i2.getASuccessor(name, _)

Copilot uses AI. Check for mistakes.

exists(ItemNode i1, ItemNode i2 |
n1 = TItemNode(i1) and
n2 = TItemNode(i2) and
i1 = getOuterScope(i2)
Copy link
Preview

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

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

The direction of the relationship appears reversed. Based on the original traitLookup logic, this should be i2 = getOuterScope(i1) to represent moving from an inner scope to an outer scope.

Suggested change
i1 = getOuterScope(i2)
i2 = getOuterScope(i1)

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant