Skip to content

Conversation

@TheVeryDarkness
Copy link
Collaborator

No description provided.

@TheVeryDarkness TheVeryDarkness changed the title Refactoring Patterns, Improve Constant Metavariables and Add More Kinds of Metavariables Refactoring Patterns, Improve Metavariables and Update Documents Jul 22, 2025
@TheVeryDarkness TheVeryDarkness requested a review from Copilot July 30, 2025 13:24

This comment was marked as outdated.

@TheVeryDarkness
Copy link
Collaborator Author

Commit b88c3c9 may be picked later

@TheVeryDarkness
Copy link
Collaborator Author

Commit b88c3c9 may be picked later

It's a bit too complex for current syntax, and I'm going to re-implement it in later commits

@TheVeryDarkness TheVeryDarkness marked this pull request as ready for review July 30, 2025 14:43
@TheVeryDarkness TheVeryDarkness requested a review from Copilot July 30, 2025 15:45
Copy link

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 metavariables handling in pattern matching, adds support for const generic parameters, and updates documentation. The key changes include introducing a Const abstraction to handle both MIR constants and parameter constants, improving pattern matching for type constants, and updating the default patterns system.

  • Refactored const variable handling to support both MIR constants and parameter constants
  • Changed the default lint level for dynamic patterns from forbid to deny
  • Added default pattern collection to eliminate the need for manual RPL_PATS configuration

Reviewed Changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/driver.rs Added handling for missing RPL_PATS environment variable and os_str_display feature
crates/rpl_constraints/src/konst.rs New Const enum to abstract over MIR constants and parameter constants
crates/rpl_meta/src/cli.rs Added collect_default_patterns() function with embedded pattern definitions
crates/rpl_interface/src/callbacks.rs Updated to use default patterns when RPL_PATS is not provided
crates/rpl_match/src/ty.rs Refactored const variable matching to handle parameter constants
tests/ui/ Updated test expectations to reflect lint level changes
Comments suppressed due to low confidence (2)

crates/rpl_constraints/src/predicates/multiple_consts.rs:19

  • The function silently returns false when constants cannot be evaluated as scalar integers, but logs a warning. This could mask real issues where the predicate should fail more explicitly.
        } else {

src/driver.rs:227

  • [nitpick] The error message uses var.display() which may not provide the most helpful output for debugging non-Unicode environment variables.
            Err(env::VarError::NotUnicode(var)) => {

macro_rules! default_pattern {
($path:literal) => {
(
PathBuf::from(concat!("/rpl/docs/patterns-pest/", $path)),
Copy link

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

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

The hardcoded absolute path "/rpl/docs/patterns-pest/" will not work correctly in different environments or installations. This should use a relative path or be configurable.

Copilot uses AI. Check for mistakes.
None
}
},
(_, _) => None,
Copy link

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

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

[nitpick] The catch-all pattern (_, _) in try_cmp_as makes it unclear what specific combinations are not handled. Consider using explicit patterns for better maintainability.

Suggested change
(_, _) => None,
(Self::MIR(_), Self::Param(_)) => None, // MIR and Param cannot be compared
(Self::Param(_), Self::MIR(_)) => None, // Param and MIR cannot be compared
(Self::MIR(_), _) => None, // MIR and other variants cannot be compared
(Self::Param(_), _) => None, // Param and other variants cannot be compared
(_, _) => None, // Catch-all for any other combinations

Copilot uses AI. Check for mistakes.
@TheVeryDarkness TheVeryDarkness merged commit 6e46dfa into RPL-Toolchain:master Aug 25, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant