Refactoring scopes
ihab...@gmail.com
Mike Samuel and I just had a discussion about what to do to clean up
our Scope implementation. A reference point here is Mike's changelist:
http://codereview.appspot.com/140066/show
* We decided that the current way of building scopes, where the
Rewriters build them downward as they go, is error prone and leads to
complexity. For example, in the above CL, Mike found a bug where
existing code was failing to call Scope.fromCatchStmt(...) at the
right place. This subtle bug passed code review.
http://codereview.appspot.com/140066/diff/4001/3032
* We concluded that the current way Rewriters use scopes -- as a scope
analysis of the input source tree *and* a convenient place to schlep
contextual information for the output tree (e.g.
getStartOfScopeStmts()) -- can be teased apart. The rewrite rules
should take a parse tree and its scope analysis as input, produce a
parse tree as output, and maintain contextual information in whatever
internal way they want.
* We need several implementations of "scope"-like objects. Some
analyses might want to build a very complete graph of symbol
definition information, while others may want only to collect the set
of variable names used in a piece of code. We will abstract that by
having the ScopeAnalyzer simply defer to a ScopeListener. To the
extent that the ScopeAnalyzer needs to maintain its own information in
order to fire messages to the ScopeListener, it will maintain this
itself and not rely on scopes constructed by the ScopeListener.
We identified a set of desired outcomes:
1. Rewriter making scopes. A Rewriter takes a parse tree as input. It
runs a scope analysis, using ScopeAnalyzer, constructing whatever
flavor of scope-like object it considers necessary for its work. It
passes this object down to its rewriter rules.
2. No partial scope. An input parse tree is treated as an immutable
unit and is scope-analyzed all at once.
3. Replace scope visitor with scope analyzer. The current scope
visitor logic in class Scope will be replaced by the ScopeAnalyzer.
4. Separate error reporting & failure decisions into a per-rewriter
client concern. The client of ScopeAnalyzer is responsible for
responding to ScopeListener events and reporting errors (e.g., about
symbols being multiply defined).
5. Have scope analyzer available to new clients. New code should be
able to use the new ScopeAnalyzer.
The plan for achieving this is:
=> Do 5. Simplify ScopeAnalyzer in CL 140066 so it does not have to
support partial scope construction. Check it in.
=> Do 2, 3 and fake-1. Refactor current class Scope to use
ScopeAnalyzer. Where the current rewriters call (say)
Scope.fromCatchStmt(), replace the implementation with a simple lookup
to return the relevant pre-constructed scope object.
=> Do 1, 4. Refactor rewriters to do scope construction. Retire class
Scope in its currently recognizable form.
Ihab
--
Ihab A.B. Awad, Palo Alto, CA