-
Notifications
You must be signed in to change notification settings - Fork 831
Add ImportResolver interface and use it for global imports #8166
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
base: main
Are you sure you want to change the base?
Conversation
84c1ad0 to
75d06c7
Compare
e40fac1 to
e1e16b8
Compare
e1e16b8 to
ae1b333
Compare
| externalInterface) | ||
| : externalInterface(externalInterface) {} | ||
|
|
||
| nullability::Nullable<Literals*> getGlobal(QualifiedName name, |
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.
Usually we just say in a comment whether a pointer can be null. I'm not sure the extra type-level wrapper is worth it. Is this useful in your experience?
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.
My thought is just to make it harder to miss by accident than a comment, and also to give us an easy way to migrate to 'real' nullability annotations in the future (we can just grep for the symbol instead of trying to find comments). Google uses nullability annotations and clang can apparently catch some bugs this way (TOTW 230 but this isn't accessible publicly).
I don't have a strong preference though, I agree that it's noisy to read.
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.
@kripken wdyt?
Add an ImportResolver interface with some implementations to handle the spec interpreter and ctor-eval