Skip to content

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented May 26, 2025

Needed in order to resolve paths inside the Core library itself, such as resolving Option at https://github.com/rust-lang/rust/blob/95a2212587f1b8500af1ac630322b0cb2e030c61/library/core/src/iter/traits/iterator.rs#L373.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label May 26, 2025
@hvitved hvitved marked this pull request as ready for review May 26, 2025 13:13
Copilot AI review requested due to automatic review settings May 26, 2025 13:13
@hvitved hvitved requested a review from a team as a code owner May 26, 2025 13:13
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 extends path resolution to include the Rust core crate itself when resolving prelude items, and updates a debug helper to point at a new test file and line.

  • Include the core crate in preludeEdge, not just its dependencies
  • Update Debug.getRelevantLocatable to match test.rs at line 74 instead of term.rs at line 71
Comments suppressed due to low confidence (2)

rust/ql/lib/codeql/rust/internal/PathResolution.qll:1441

  • The filename pattern was changed from %/term.rs to %/test.rs; verify that the target file is actually named test.rs. If the intention was to match term.rs, update the pattern accordingly.
filepath.matches("%/test.rs") and

rust/ql/lib/codeql/rust/internal/PathResolution.qll:1442

  • The startline comparison changed from a list literal [71] to a single integer 74. Ensure this aligns with the predicate’s expected type: if startline can have multiple values, wrap 74 in a list.
startline = 74

private predicate preludeEdge(SourceFile f, string name, ItemNode i) {
exists(Crate core, ModuleLikeNode mod, ModuleItemNode prelude, ModuleItemNode rust |
f = any(Crate c0 | core = c0.getDependency(_)).getASourceFile() and
f = any(Crate c0 | core = c0.getDependency(_) or core = c0).getASourceFile() and
Copy link

Copilot AI May 26, 2025

Choose a reason for hiding this comment

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

[nitpick] The combined dependency check core = c0.getDependency(_) or core = c0 can be hard to read; consider adding parentheses around the OR expression or splitting it into two predicates for clarity.

Suggested change
f = any(Crate c0 | core = c0.getDependency(_) or core = c0).getASourceFile() and
f = any(Crate c0 | (core = c0.getDependency(_) or core = c0)).getASourceFile() and

Copilot uses AI. Check for mistakes.
@hvitved hvitved requested a review from paldepind May 26, 2025 17:17
@hvitved hvitved merged commit 78b9d8f into github:main May 28, 2025
16 checks passed
@hvitved hvitved deleted the rust/path-resolution-prelude-in-core branch May 28, 2025 08:53
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.

2 participants