Bundle getter/setter names for lexical lookups #548
Conversation
…es in Identifier Reference
eernstg
changed the title
eernstg
changed the title
There was a problem hiding this comment.
Thanks for looking at this! This basically looks good to me. The one really substantive issue I have is around the definition of lexical scopes with respect to imports and exports. The implementations are inconsistent, and I don't feel like this rewrite pins down which one is incorrect. Is this just something that is already unspecified now, and we're just not (yet) resolving it? Or am I missing it?
There was a problem hiding this comment.
Review response. I think all comments have been resolved, but I kept the complex one open, namely: the one about considering imports/exports as a mechanism that could be specified in terms of scope nesting. I think we shouldn't do that, because we'd need a notion of namespace union + conflict resolution, and there's no need to pollute the rules about lexically nested scopes with such a concept.
…iable"; also changed "getter" to "library getter" where an ambiguity could arise
…form a setter lookup and find a local variable
…tters/getters, plus commentary about implicitly induced getters/setters)
…uced "has access to this" to avoid duplication
|
LGTM |
…ked up lexically)
This PR updates the language specification such that it specifies "bundling" of getters and setters for all parts of Dart except imports and exports, that is, whenever an identifier is looked up in the enclosing lexical scopes.
It defines a new concept of 'lexical lookup' which will look for
idas well asid=and select the lexically nearest declaration with one of those names, and make it an error if we are looking upidand the selected scope has anid=but noid, and vice versa; or if we look upid/id=and nothing is in scope, conclude that we must use an instance member, and the class interface doesn't have the required getter-or-setter, and so on. It's a substantial rewrite, clarification, and bug fix.In any case, the newly specified approach is what we already did/intended for 'Identifier Reference'.
It is also what the CFE already does for 'Unqualified Invocation'. (In this case, the spec required a simple search for
idand ignored conflicts withid=in intermediate scopes, but the new text and the CFE behavior is more consistent with the rest of the language.)For assignments, we used to spec a nearly consistent search for
idas well asid=, and tools are actually slightly more consistent (in their bundling of getters and setters, as specified now).All in all, this should not be a breaking change in any other way than for unqualified invocation.