-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Make Access syntax fail to compile for unsupported values #4076
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
unless is_atom(key) do | ||
raise ArgumentError, | ||
"the Access for keywords expect the key to be an atom, got: " <> inspect(key) | ||
end |
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 be cleaner to move this to its own clause.
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've made it at first place but decided to keep list
-related logic grouped, but have no strong feelings one way or the other. 😄
bec1a7b
to
2f68c0a
Compare
rewrite(?access, _DotMeta, 'get', Meta, [Arg, _], Env) | ||
when ?is_literal(Arg) orelse element(1, Arg) == '{}' orelse element(1, Arg) == '<<>>' -> | ||
elixir_errors:compile_error(Meta, ?m(Env, file), | ||
"the Access is not available for the value: ~ts", ['Elixir.Macro':to_string(Arg)]); |
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.
"the Access syntax"
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.
But this place handles not only Access syntax []
but also actual Access calls.
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.
It will be alright
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.
Or if you want, be explicit: "the Access syntax and calls to Access.get/2 are not available for..."
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.
It could be strange to say "Access syntax" for Access.get(1, :foo)
call.
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.
"the Access syntax and calls to Access.get/2 are not available for the value: 1"
I've added just tiny comments, |
Make Access syntax fail to compile for unsupported values
Closes #4070.