Add escape/1 and use it to fix Regex escaping in OTP28.1 #14720

sabiwara · 2025-08-21T12:30:28Z

Take 2 for #14719.

Leverages :re.import added in erlang/otp#9976.

Here I went for Macro.escape rather than :elixir_expand, because :elixir_expand is harder to work with since we're already in AST land and the AST for a struct is hard to pattern-match on.

This works since Macro.escape is used in ~r, and also fixes the escaping issue discussed previously. But this is violating the assumption that escape emits AST for a literal, which is probably a no-go.

Will work on supporting the :export option separately since this is not strictly needed for this.

josevalim · 2025-08-23T08:38:39Z

Notes to self: how we're going to check that the __escape__ function is defined?

function_exported? is not enough because there is no guarantee the struct module will be loaded. For example, module Bar.foo may do %Foo{} which is inlined as a map, so when calling Bar.foo, the Foo module is not loaded
Code.ensure_loaded? should be enough, however. If there is a struct, then definition its module is available
Code.ensure_compiled? is problematic, as it can lead to deadlocks in the compiler

erlang/otp#9976

sabiwara · 2025-08-30T02:43:17Z

lib/elixir/lib/regex.ex

+        # OTP 28.0 works in degraded mode performance-wise, we need to recompile from the source
+        true ->
+          quote do
+            {:ok, pattern} =
+              :re.compile(unquote(Macro.escape(regex.source)), unquote(Macro.escape(regex.opts)))
+
+            pattern
+          end
+      end


Was wondering. Perhaps we could/should warn here, so that people notice and pro-actively bump to OTP28.1?

Yes, warning sounds good to me!

Hum I tried e1907ea but it seems to cause some bootstrapping issues.

Not too familiar with IO.warn_once, maybe will give up for now 😅

So let's warn on boot in elixir.erl. Check if version is >= 28 and the import function is not available.

This seems to work, but would break too many integration tests in the CI?
d1536b4

@sabiwara I think for CI we will run on 28.1. So perhaps we add the warning in a separate PR, which we will merge once we move CI to 28.1. WDYT?

Adding the warning separately sounds good! 💜

sabiwara · 2025-08-30T02:46:43Z

lib/elixir/src/elixir_quote.erl

+  maybe
+    #{'__struct__' := Module} ?= Map,
+    true ?= is_atom(Module),
+    {module, Module} ?= code:ensure_loaded(Module),
+    true ?= erlang:function_exported(Module, '__escape__', 1),
+    Module:'__escape__'(Map)
+  else
+    _ ->
+      TT = [escape_map_key_value(K, V, Map, Q) || {K, V} <- lists:sort(maps:to_list(Map))],
+      {'%{}', [], TT}
+  end;


Note: We can finally use maybe 🎉
It fits perfectly here.

Since we're requiring OTP26, there is no runtime flag anymore, just the module one.

josevalim · 2025-08-30T05:56:42Z

lib/elixir/test/elixir/macro_test.exs

      end
    end
+
+    @tag skip: not function_exported?(:re, :import, 1)


Let's use exclude in the test/test_helper.exs instead, for consistency? We should probably do Code.ensure_loaded?(:re) as well...

Do you mean adding an otp_version_exclude here?
https://github.com/elixir-lang/elixir/blob/main/lib/elixir/test/elixir/test_helper.exs#L138-L139

I'm slightly concerned that the extra indirection would make it harder to follow, esp. if it is just used at one place?
But happy to do it if you think that's best.

We should probably do Code.ensure_loaded?(:re) as well.

Indeed, sorry about that. I'm starting to wish we had one function for this as suggested recently 😅

I would add one called re_import_exclude, yes, so we have all of them handled in a single place instead of scattered.

sabiwara · 2025-08-30T06:12:42Z

I think we're good to go!

I was even able to apply it to a fine-based project I'm working on, which also has runtime refs, a text format and a binary format - it worked beautifully ✨
https://codeberg.org/sabiwara/gaia/src/branch/main/lib/gaia.ex#L313-L321
https://codeberg.org/sabiwara/gaia/src/branch/main/test/gaia_test.exs#L16-L25

josevalim · 2025-08-30T06:20:53Z

Agreed, we just need docs and we can ship it! :)

sabiwara · 2025-08-30T06:28:04Z

Agreed, we just need docs and we can ship it! :)

What's a good place for it? Macro.escape?

josevalim · 2025-08-30T06:32:39Z

Macro.escape is a good call for now, yes!

sabiwara · 2025-08-30T08:44:38Z

Pushed some docs. It's a bit of a complex topic, which makes it tricky to explain simply.
Improvements and suggestions are welcome 🙂

lib/elixir/lib/macro.ex

josevalim

I added a comment with a slightly different spin on the documentation. Feel free to use it or discard it as you prefer.

Co-authored-by: José Valim <[email protected]>

sabiwara · 2025-08-30T08:59:37Z

I added a comment with a slightly different spin on the documentation. Feel free to use it or discard it as you prefer.

I love it 💜

lib/elixir/lib/macro.ex

sabiwara · 2025-08-30T09:17:05Z

@josevalim OK to backport right? (since it's from 1.19)

josevalim · 2025-08-30T09:26:45Z

Yes, 100%.

Leverages newly added :re.import/1. erlang/otp#9976

sabiwara · 2025-08-30T09:30:44Z

Done https://github.com/elixir-lang/elixir/tree/v1.19 ✅

sabiwara mentioned this pull request Aug 21, 2025

Use new OTP28.1 :re.import for regexes #14719

Closed

sabiwara marked this pull request as draft August 22, 2025 13:34

sabiwara mentioned this pull request Aug 23, 2025

Expand-compile regexes using OTP 28.1 :re.import #14722

Closed

Use new OTP28.1 :re.import in escaped regex AST

f47e748

erlang/otp#9976

sabiwara force-pushed the import-re2 branch 2 times, most recently from 14f9002 to 6cb4b82 Compare August 30, 2025 02:41

sabiwara added 2 commits August 30, 2025 11:42

Without cheating: defining __escape__/1

bf77434

Remove special escape clause for module attribute

602096f

sabiwara force-pushed the import-re2 branch from 6cb4b82 to 602096f Compare August 30, 2025 02:42

sabiwara commented Aug 30, 2025

View reviewed changes

josevalim reviewed Aug 30, 2025

View reviewed changes

josevalim approved these changes Aug 30, 2025

View reviewed changes

sabiwara marked this pull request as ready for review August 30, 2025 06:12

sabiwara force-pushed the import-re2 branch from 6b71f6c to e1907ea Compare August 30, 2025 06:29

Move exclude tag to test helper

6ef94c5

sabiwara force-pushed the import-re2 branch from e1907ea to d1536b4 Compare August 30, 2025 08:05

Document __escape__/1 in Macro.escape/2 docs

c3be329

sabiwara force-pushed the import-re2 branch from 5eaecd6 to c3be329 Compare August 30, 2025 08:46

sabiwara mentioned this pull request Aug 30, 2025

Warn on boot for OTP28.0 #14732

Draft

josevalim reviewed Aug 30, 2025

View reviewed changes

lib/elixir/lib/macro.ex Outdated Show resolved Hide resolved

josevalim approved these changes Aug 30, 2025

View reviewed changes

Rephrase documentation

f490780

Co-authored-by: José Valim <[email protected]>

Minor doc tweaks

5814e03

krns reviewed Aug 30, 2025

View reviewed changes

lib/elixir/lib/macro.ex Outdated Show resolved Hide resolved

Minor doc tweak

8b0e498

sabiwara changed the title ~~Use new OTP28.1 :re.import in escaped regex AST~~ Define __escape__/1 and use it to fix Regex escaping in OTP28.1 Aug 30, 2025

sabiwara changed the title ~~Define __escape__/1 and use it to fix Regex escaping in OTP28.1~~ Add __escape__/1 and use it to fix Regex escaping in OTP28.1 Aug 30, 2025

sabiwara merged commit 7a8dc78 into elixir-lang:main Aug 30, 2025
13 checks passed

sabiwara deleted the import-re2 branch August 30, 2025 09:15

sabiwara added a commit that referenced this pull request Aug 30, 2025

Add __escape__/1 and use it to fix Regex escaping in OTP28.1+ (#14720)

ff21a9d

Leverages newly added :re.import/1. erlang/otp#9976

This was referenced Aug 30, 2025

Regular expression export/import erlang/otp#9976

Merged

Shallow-validate the return of __escape__/1 #14736

Merged

Add __escape__/1 and use it to fix Regex escaping in OTP28.1 #14720

Add __escape__/1 and use it to fix Regex escaping in OTP28.1 #14720

Uh oh!

Conversation

sabiwara commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josevalim commented Aug 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sabiwara commented Aug 30, 2025

Uh oh!

josevalim commented Aug 30, 2025

Uh oh!

sabiwara commented Aug 30, 2025

Uh oh!

josevalim commented Aug 30, 2025

Uh oh!

sabiwara commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

josevalim left a comment

Choose a reason for hiding this comment

Uh oh!

sabiwara commented Aug 30, 2025

Uh oh!

Uh oh!

Uh oh!

sabiwara commented Aug 30, 2025

Uh oh!

josevalim commented Aug 30, 2025

Uh oh!

sabiwara commented Aug 30, 2025

Uh oh!

Uh oh!

Add escape/1 and use it to fix Regex escaping in OTP28.1 #14720

Add escape/1 and use it to fix Regex escaping in OTP28.1 #14720

sabiwara commented Aug 21, 2025 •

edited

Loading

sabiwara commented Aug 30, 2025 •

edited

Loading