Skip to content

Fix dialyzer opaqueness warnings on module attrs in OTP28#14755

Merged
sabiwara merged 2 commits intoelixir-lang:mainfrom
sabiwara:generated-escape
Sep 9, 2025
Merged

Fix dialyzer opaqueness warnings on module attrs in OTP28#14755
sabiwara merged 2 commits intoelixir-lang:mainfrom
sabiwara:generated-escape

Conversation

@sabiwara
Copy link
Copy Markdown
Contributor

@sabiwara sabiwara commented Sep 9, 2025

Marks module attributes as generated in case they contain opaque terms

Close #14750

To be backported to 1.19

@sabiwara sabiwara changed the title Mark module attributes as generated in case they contain opaque terms Fix dialyzer opaqueness warnings on module attributes in OTP28 Sep 9, 2025
@sabiwara sabiwara changed the title Fix dialyzer opaqueness warnings on module attributes in OTP28 Fix dialyzer opaqueness warnings on module attrs in OTP28 Sep 9, 2025
@josevalim
Copy link
Copy Markdown
Member

I wonder if instead we should do Macro.escape(..., generated: true)? I can see this being handy in other occasions. But this PR also works as is!

@sabiwara
Copy link
Copy Markdown
Contributor Author

sabiwara commented Sep 9, 2025

I wonder if instead we should do Macro.escape(..., generated: true)? I can see this being handy in other occasions. But this PR also works as is!

I thought about it too, yeah it sounds good!
I was worried about bootstrapping issues but it seems to work just fine. Will push.

@sabiwara
Copy link
Copy Markdown
Contributor Author

sabiwara commented Sep 9, 2025

c351990

Comment thread lib/elixir/lib/macro.ex
Comment on lines +811 to +813
* `:generated` - (since v1.19.0) Whether the AST should be considered as generated
by the compiler or not. This means the compiler and tools like Dialyzer may not
emit certain warnings.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread lib/elixir/lib/macro.ex
Comment on lines +920 to +921
{caller, meta, args} when generated and is_list(meta) ->
{caller, [generated: true] ++ meta, args}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we wrap with a __block__ for other ASTs?
It feels a bit too much, we shouldn't need it for this use case and quote doesn't do it either.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there is no need, yeah.

@sabiwara sabiwara merged commit e2f6bba into elixir-lang:main Sep 9, 2025
13 checks passed
@sabiwara sabiwara deleted the generated-escape branch September 9, 2025 08:51
sabiwara added a commit that referenced this pull request Sep 9, 2025
* Mark module attributes as generated in case they contain opaque terms

* Add and use Macro.escape(ast, generated: true)
@sabiwara
Copy link
Copy Markdown
Contributor Author

sabiwara commented Sep 9, 2025

Backported ✅

Comment thread lib/elixir/lib/macro.ex
nodes. Note this option changes the semantics of escaped code and
it should only be used when escaping ASTs. Defaults to `false`.

* `:generated` - (since v1.19.0) Whether the AST should be considered as generated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sabiwara the new options should be included in escape_opts type

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wooops, thank you for catching this one ! 💜
Will fix

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed and backported #14761

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Was this backported to Elixir 1.19? It still appears to be happening on elixir 1.19.5 and otp 28.3.2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@sabiwara i'm still having the issue at Elixir 1.19.5 with otp 28.4.1. Should I upgrate to recent versions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@itsGabriel @hez this PR has been backported to 1.19.5, but wasn't enough to fix all instances of #14837 due to the way OTP28 contract checking ignores generated annotations.

An example of this is:

  @map_set MapSet.new(1..10)
  @spec contract_violation :: MapSet.t()
  def contract_violation, do: @map_set

raises a contract_with_opaque on 1.19.5 / OTP28:

file:line:contract_with_opaque
The @spec for Repro.contract/0 has an opaque
subtype %MapSet{:map => MapSet.internal(_)} which is violated by the success typing.

We ended up removing @opaque from MapSet or URI types, but this part was done after 1.19.5 and hasn't been backported.

The 1.20 rc should fix this, but until the stable release, you can suppress it by temporarily adding @dialyzer {:no_opaque, [fun: arity]} (or just @dialyzer :no_opaque).

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.

Opaqueness warning on OTP28

5 participants