Skip to content

Conversation

ya7010
Copy link
Collaborator

@ya7010 ya7010 commented Oct 19, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings October 19, 2025 14:45
@ya7010 ya7010 enabled auto-merge October 19, 2025 14:45
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

Refactors the regex! macro to require an explicit static keyword in its invocation.

  • Changes macro pattern to ($(static $var = $re);+;) form.
  • Updates all macro invocations to include static before each identifier.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


macro_rules! regex {
($($var:ident = $re:expr);+;) => {
($(static $var:ident = $re:expr);+;) => {
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

Requiring static in the macro invocation introduces a breaking change to the macro's call-site API without adding semantic value (the macro already emits static). Consider adding an additional macro arm to accept the previous form ($($var:ident = $re:expr);+;) for backward compatibility, or remove static from the invocation pattern and keep it only in the expansion.

Copilot uses AI. Check for mistakes.

REGEX_FLOAT = r"^[0-9_]+(:?(:?\.[0-9_]+)?[eE][+-]?[0-9_]+|\.[0-9_]+)$";
REGEX_IS_DATE_TIME = r"^[0-9]{4}-[0-9]{2}-[0-9]{2}";
REGEX_OFFSET_DATE_TIME =
static REGEX_INTEGER_BIN = r"^0b[0|1](:?_?[0|1])*$";
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

Character class [0|1] includes '|' as a valid character; it should be [01] to restrict to binary digits. Similarly adjust the second occurrence inside (:??[0|1]) to (:??[01]). Proposed: r"^0b01*$".

Suggested change
static REGEX_INTEGER_BIN = r"^0b[0|1](:?_?[0|1])*$";
static REGEX_INTEGER_BIN = r"^0b[01](?:_?[01])*$";

Copilot uses AI. Check for mistakes.

Comment on lines +24 to +28
static REGEX_INTEGER_BIN = r"^0b[0|1](:?_?[0|1])*$";
static REGEX_INTEGER_OCT = r"^0o[0-7](:?_?[0-7])*$";
static REGEX_INTEGER_HEX = r"^0x[0-9A-Fa-f](:?_?[0-9A-Fa-f])*$";
static REGEX_INTEGER_DEC = r"^(:?[1-9](:?_?[0-9])*|0)$";
static REGEX_FLOAT = r"^[0-9_]+(:?(:?\.[0-9_]+)?[eE][+-]?[0-9_]+|\.[0-9_]+)$";
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

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

The sequences (:? ... ) and :?(:? ... ) appear intended as non-capturing groups but are malformed; Rust's regex crate uses (?: ... ). Correct forms: REGEX_INTEGER_OCT r"^0o0-7$", REGEX_INTEGER_HEX r"^0x0-9A-Fa-f$", REGEX_INTEGER_DEC r"^(?:1-9*|0)$", REGEX_FLOAT r"^[0-9_]+(?:(?:\.[0-9_]+)?[eE][+-]?[0-9_]+|\.[0-9_]+)$". (Binary line also needs fixes noted separately.)

Suggested change
static REGEX_INTEGER_BIN = r"^0b[0|1](:?_?[0|1])*$";
static REGEX_INTEGER_OCT = r"^0o[0-7](:?_?[0-7])*$";
static REGEX_INTEGER_HEX = r"^0x[0-9A-Fa-f](:?_?[0-9A-Fa-f])*$";
static REGEX_INTEGER_DEC = r"^(:?[1-9](:?_?[0-9])*|0)$";
static REGEX_FLOAT = r"^[0-9_]+(:?(:?\.[0-9_]+)?[eE][+-]?[0-9_]+|\.[0-9_]+)$";
static REGEX_INTEGER_BIN = r"^0b[01](?:_?[01])*$";
static REGEX_INTEGER_OCT = r"^0o[0-7](?:_?[0-7])*$";
static REGEX_INTEGER_HEX = r"^0x[0-9A-Fa-f](?:_?[0-9A-Fa-f])*$";
static REGEX_INTEGER_DEC = r"^(?:[1-9](?:_?[0-9])*|0)$";
static REGEX_FLOAT = r"^[0-9_]+(?:(?:\.[0-9_]+)?[eE][+-]?[0-9_]+|\.[0-9_]+)$";

Copilot uses AI. Check for mistakes.

@ya7010 ya7010 merged commit d5391d7 into main Oct 19, 2025
8 checks passed
@ya7010 ya7010 deleted the refactor_macro branch October 19, 2025 14:56
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