Skip to content

Conversation

sabiwara
Copy link
Contributor

Following on #14720, I've been thinking if we could/should improve the error message when trying to use regex module attributes in guards/match scope.
This PR explores the idea, but feel free to close if this is overthinking on my part.

Before (confusing: :re.compile mentioned but not in the code)
Screenshot 2025-09-20 at 10 15 31

After
Screenshot 2025-09-20 at 10 16 06

## Shared functions

@doc false
defmacro __assert_assert_no_match_or_guard_scope__(exp) do
Copy link
Member

Choose a reason for hiding this comment

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

I think in this case some code duplication is fine. Let's just reimplement the error message directly in the Regex module, since that's what we would expect library authors to do too.

Comment on lines 1021 to 1022
__assert_assert_no_match_or_guard_scope__("an escaped Regex")
:re.import(unquote(Macro.escape(exported)))
Copy link
Member

Choose a reason for hiding this comment

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

Instead of generating additional code, we should call Regex.__import__(unquote(...)), where __import__ itself is a macro which also checks the environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sounds much better indeed 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arg, this might actually not work, since unlike Kernel we need to require Regex.

       quote do
            require Regex
            Regex.__import_pattern__(unquote(Macro.escape(exported)))
          end

Which then makes it impossible to compile elixir:

==> elixir (compile)
error: you are trying to use/import/require the module Regex which is currently being defined.

(although it seems weird to warn here, since it's happening within quote 🤔)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed the broken commit just in case: c1dee89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants