-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix dialyzer opaqueness warnings on module attrs in OTP28 #14755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I wonder if instead we should do |
I thought about it too, yeah it sounds good! |
* `: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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically the same as https://hexdocs.pm/elixir/1.19.0-rc.0/Macro.html#t:metadata/0
{caller, meta, args} when generated and is_list(meta) -> | ||
{caller, [generated: true] ++ meta, args} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
fd96bab
to
c351990
Compare
* Mark module attributes as generated in case they contain opaque terms * Add and use Macro.escape(ast, generated: true)
Backported ✅ |
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed and backported #14761 ✅
Marks module attributes as generated in case they contain opaque terms
Close #14750
To be backported to 1.19