Skip to content

Rust: extract hasImplementation on functions and consts #19649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 12, 2025

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Jun 3, 2025

When skipping bodies in library code, we were losing the information whether a body was originally present. This can be important, for example when determining whether a trait method has a default implementation.

With this change that information can now be recovered via the hasImplementation predicate.

Finally, the above uncovered a bug in the rustgen templates, where a detached predicate property would not be handled correctly. That is fixed now.

Paolo Tranquilli added 4 commits June 3, 2025 10:41
When skipping bodies in library code, we lose the information whether a
body was originally present. This can be important, for example when
determining whether a trait method has a default implementation.

With this change that information can be recovered via the
`hasImplementation` predicate.
@github-actions github-actions bot added the Rust Pull requests that update Rust code label Jun 3, 2025
@redsun82 redsun82 force-pushed the redsun82/rust-has-implementation branch from f88949a to 1110fea Compare June 3, 2025 10:12
@redsun82 redsun82 changed the title Rust: extract hasImplementation on functions Rust: extract hasImplementation on functions and consts Jun 3, 2025
Base automatically changed from redsun82/rust-extract-libs to main June 12, 2025 08:45
@redsun82 redsun82 marked this pull request as ready for review June 12, 2025 13:26
@Copilot Copilot AI review requested due to automatic review settings June 12, 2025 13:26
@redsun82 redsun82 requested review from a team as code owners June 12, 2025 13:26
@redsun82 redsun82 requested review from hvitved and aibaars June 12, 2025 13:27
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 adds a has_implementation predicate to capture whether functions and constants originally had bodies (even when skipped in library code), updates the extractor to emit this information, and adds schema, upgrade/downgrade scripts, and codegen template fixes to support the new tables.

  • Define has_implementation in annotations.py for both Const and Function
  • Emit the new predicate in the extractor (base.rs) when a body is present
  • Add new tables in rust.dbscheme, upgrade/downgrade scripts, update .generated.list, and fix the codegen template for detached predicates

Reviewed Changes

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

Show a summary per file
File Description
rust/schema/annotations.py Add has_implementation predicate with docs for Const and Function
rust/extractor/src/translate/base.rs Call emit_*_has_implementation when bodies exist
rust/ql/lib/rust.dbscheme Introduce const_has_implementation and function_has_implementation tables
rust/ql/lib/upgrades/.../upgrade.ql & upgrade.properties (and downgrades) Backfill and remove the new tables via QL upgrade/downgrade scripts
rust/ql/.generated.list Refresh generated file hashes for updated QLL files
misc/codegen/templates/rust_classes.mustache Update template to generate predicate emit methods correctly
Comments suppressed due to low confidence (2)

rust/extractor/src/translate/base.rs:774

  • There are currently no extractor tests validating that emit_function_has_implementation and emit_const_has_implementation fire correctly when bodies are present or absent. Consider adding tests covering both scenarios to ensure the new predicates are populated as expected.
if node.body().is_some() {

rust/ql/lib/rust.dbscheme:2993

  • Consider adding a brief comment above the function_has_implementation and const_has_implementation table definitions to describe their purpose and improve maintainability of the DB schema.
#keyset[id]
function_has_implementation(

aibaars
aibaars previously approved these changes Jun 12, 2025
@redsun82
Copy link
Contributor Author

sorry @aibaars, I had forgotten to update a test expectation, can you reapprove?

@redsun82 redsun82 merged commit a5dba9b into main Jun 12, 2025
26 checks passed
@redsun82 redsun82 deleted the redsun82/rust-has-implementation branch June 12, 2025 15:19
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.

3 participants