`unquote` fails to compile in patterns and guards since 1.18

[lukaszsamson]

Existing issue

• I have searched existing issues and could not find a duplicate.

Elixir and Erlang/OTP versions

Erlang/OTP 28 [erts-16.4.0.1] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit]

Elixir 1.20.1 (compiled with Erlang/OTP 28)

Affected versions: all since 1.18.0

Operating system

any

Current behavior

Since cc68ad9 all unquoted expressions are wrapped in runtime validation call but the validation is not allowing unquotes

Repro:

defmodule SchemaAst do
  def extract_defaulted_field(ast) do
    case ast do
      quote(do: {:field, unquote(name), default: 1}) -> {:ok, name}
      _ -> :error
    end
  end
end

on 1.18.0 - 1.20.1

error: cannot invoke remote function :elixir_quote.unquote/1 inside a match
└─ iex:4: SchemaAst.extract_defaulted_field/1

** (CompileError) iex: cannot compile module SchemaAst (errors have been logged)
    (elixir 1.20.1) src/elixir_expand.erl:659: :elixir_expand.expand_list/5
    (elixir 1.20.1) src/elixir_expand.erl:743: :elixir_expand.mapfold/5
    (elixir 1.20.1) src/elixir_expand.erl:25: :elixir_expand.expand/3
    (elixir 1.20.1) src/elixir_expand.erl:273: :elixir_expand.expand/3
    (elixir 1.20.1) src/elixir_expand.erl:731: :elixir_expand.expand_args/3
    (elixir 1.20.1) src/elixir_clauses.erl:169: :elixir_clauses.match/6
    iex:1: (file)

On 1.17.3

{:module, SchemaAst,
 <<70, 79, 82, 49, 0, 0, 6, 20, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 0, 209,
   0, 0, 0, 21, 16, 69, 108, 105, 120, 105, 114, 46, 83, 99, 104, 101, 109, 97,
   65, 115, 116, 8, 95, 95, 105, 110, 102, ...>>, {:extract_defaulted_field, 1}}
SchemaAst.extract_defaulted_field({:{}, [], [:field, {:asd, [], nil}, [default: 1]]})
{:ok, {:asd, [], nil}}

The same applies for guard context. There were no breaking changes announced in 1.18 changelog and unquote stays supported in patterns and guards.

Expected behavior

Runtime validation should permit unquotes or if for some reason this is no longer supported it should be error with a nice message and be mentioned in as breaking change in changelog

[josevalim]

Thanks, I agree the plain unquote(name) case regressed here. However, not every unquote form worked in this context before. For example, unquote_splicing in lists already goes through runtime quote callbacks such as :elixir_quote.list/2 / tail_list/3, which cannot appear in a match or guard. Dot-unquote has a separate runtime callback path too.

So even if we supported it, it would be partial support, and I am not sure if we should have this partial state.

Thoughts?

[sabiwara]

Oh, I wasn't aware of this use case at all when I added this. The fact it didn't came up within 3 major versions though, lets me think it might never have been a thing though?

Besides the limitations listed above, I think the approach should not be recommended from a semantic perspective because of meta:

# expanded pattern
{:{}, [], [:field, name, [default: 1]]} ->

# what should be written
{:{}, _, [:field, name, [default: 1]]} ->

But perhaps quote should have an extra option set to :guard or :pattern, and in the case of :pattern we could replace meta by wildcards and remove the runtime validation (and straight-up refuse unquote_splicing ). This would make the feature much more practical?
Edit: Maybe not that easy with things like no_parens though. 🤔

[josevalim]

The fact it didn't came up within 3 major versions though, lets me think it might never have been a thing though?

That's a good call out. It is also worth noting that this specifically happens when you have both quote and unquote in the pattern. A macro that does this:

quote do
  case x do
    {:foo, unquote(bar)} -> ...
  end
end

still works. So my suggestion would be:

1. Make this raise a better error message, perhaps by adding a elixir_quote.unquote == raise version
2. Update the CHANGELOG in v1.18, both in the tag and in the original release notes

Thoughts @sabiwara?

[sabiwara]

1. Make this raise a better error message, perhaps by adding a elixir_quote.unquote == raise version

@josevalim makes sense, looking into it!

2. Update the CHANGELOG in v1.18, both in the tag and in the original release notes

Given the three-major-versions argument and the fact as you said it's only when both quote and unquote are in the pattern (a non-documented use case to begin with), I'd say it might be a bit overkill?

@lukaszsamson was this from an actual project or found with fuzzing?

[josevalim]

I’d say we would have to document it anyway, in case someone runs into it and, if this came from fuzzying/LLMs, we don’t get new reports :D

[sabiwara]

@josevalim OK! I never did it, do we do it for all tags from 1.18.0 ~ 1.18.4? Just push the tags?

4. Hard deprecations
   Elixir

• [Kernel] Using unquote is deprecated when quote is used inside a pattern or guard

WDYT?

[josevalim]

Don’t change the tags. Just the v1.18 branch on GitHub and optionally the release notes on GitHub. But now that I think about it, better not in case it breaks or triggers some automation. So just the branch is fine!

[josevalim]

Oh, and call it ## Potential breaking changes.

[lukaszsamson]

Found by fuzzing so likely no reallife users of this pattern. My thinking was that quotes are deliberately allowed in patterns and guards so I found it strange that unquote stopped working

[sabiwara]

Don’t change the tags. Just the v1.18 branch on GitHub and optionally the release notes on GitHub. But now that I think about it, better not in case it breaks or triggers some automation. So just the branch is fine!

OK makes more sense, you previously said "tag" which sounded wrong 😄

[sabiwara]

ef36b16 ✅