-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Rust: Type inference for macro expressions #19751
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for inferring types from macro expressions in the Rust CodeQL type inference engine.
- Extended the
typeEquality
predicate to treatMacroExpr
expansions as equivalent nodes. - Added a new
macros
test module with aformat!
invocation inmain.rs
. - Updated the
type-inference.expected
file to include expected entries for macro expressions and their expansions.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
rust/ql/lib/codeql/rust/internal/TypeInference.qll | Added an or clause in typeEquality to equate macro expressions with their expansions. |
rust/ql/test/library-tests/type-inference/main.rs | Introduced a macros module and a format! call to exercise macro-based type inference. |
rust/ql/test/library-tests/type-inference/type-inference.expected | Updated expected output to list MacroExpr nodes and their expanded types for the tests. |
Comments suppressed due to low confidence (2)
rust/ql/test/library-tests/type-inference/main.rs:1808
- [nitpick] The module name
macros
could be confused with Rust's built-in macros. Consider renaming it to something more descriptive likemacro_tests
ormacro_expr_tests
.
mod macros {
rust/ql/test/library-tests/type-inference/main.rs:1810
- This inline TODO references an open PR. Remove or update this comment once that change is merged to avoid stale test markers.
let x = format!("Hello, {}", "World!"); // $ MISSING: type=x:String -- needs https://github.com/github/codeql/pull/19658
@@ -265,6 +265,8 @@ private predicate typeEquality(AstNode n1, TypePath prefix1, AstNode n2, TypePat | |||
n1 = be.getLhs() and | |||
n2 = be.getRhs() | |||
) | |||
or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider adding a brief comment above this clause explaining why macro expressions require special handling here, improving future maintainability.
or | |
or | |
// Macro expressions require special handling because they can expand into multiple | |
// expressions or statements, which affects type inference. |
Copilot uses AI. Check for mistakes.
60ea89b
to
2f698d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
DCA shows a nice 1 percentage point increase in resolvable calls.