Conversation
Gas ChangesNo changes detected. Summary
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d41ed683a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
📊 Bytecode Size Changes (venom)No changes detected. Full bytecode sizes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9b286a3f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if prev_decl is None: | ||
| msg += f" as a {prev}" | ||
| msg = f"'{attr}' is already the name of a built-in" |
There was a problem hiding this comment.
Don't classify JSON ABI imports as built-ins
When the previous namespace entry is a JSON ABI import, this branch still reports the collision as a built-in: the .json import path returns InterfaceT.from_json_abi(...), and that factory builds the interface with decl_node=None, so import foo as X followed by another X declaration or alias now emits "'X' is already the name of a built-in". The fresh evidence after the earlier import fix is that only .vy imports were wrapped with an alias decl node; JSON ABI imports still have no declaration node, so this heuristic remains inaccurate for them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think the fix for this would be to populate decl_node for things comming from JSON ABIs, as this would also allow us to provide location for error messages involving members from json abis
What I did
Improve error messages when trying to shadow a built-in:
Before:
After:
How I did it
When there is a name collision, check if the previous definition has a source location, if it doesn't: it must be a built-in.
How to verify it
pytestNew tests added
Commit message
Description for the changelog
Improve error message when trying to shadow a built-in
Cute Animal Picture