Step back and remove more getNode() invocations

Step back and remove more getNode() invocations

In many cases we need Element.getNode() only for producing error.
This CL changes these places and well as other places where we can easily use only Elements without getNode().


R=zundel@google.com,brianwilkerson@google.com
BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=5349

[Brian Wilkerson, 2012-03-12 19:09:26]

LGTM

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/Elements.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Elements.java:444: public static
SourceInfo getNameLocation(Element element) {
In a subsequent CL this method should go away.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File
compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java
(right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java:51:
// TODO(jgw): Pass in modifiers directly, not referencing node.
Given that we're using the node for other things now I think we can remove this
TODO.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java:94:
public boolean hasBody() {
As I said in the previous CL, I think we want to compute this in the constructor
and store it in a field (unless we think this will go away later).

[scheglov, 2012-03-12 19:36:24]

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File
compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java
(right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java:51:
// TODO(jgw): Pass in modifiers directly, not referencing node.
On 2012/03/12 19:09:26, Brian Wilkerson wrote:
> Given that we're using the node for other things now I think we can remove
this
> TODO.

Done.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/MethodElementImplementation.java:94:
public boolean hasBody() {
On 2012/03/12 19:09:26, Brian Wilkerson wrote:
> As I said in the previous CL, I think we want to compute this in the
constructor
> and store it in a field (unless we think this will go away later).

Done.

[zundel, 2012-03-12 19:42:31]

Well, I still haven't had a time to look at it all but its already committed so
I thought I'd send you my comments.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/LibraryDepsVisitor.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/LibraryDepsVisitor.java:48: Element
enclosing = target.getEnclosingElement();
Why the removal of EnclosingElement everywhere?  You have removed many
references to it but it still exists in the class hierarchy.  Part of the JDK6
bug?

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/parser/DartScannerParserContext.java
(right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/parser/DartScannerParserContext.java:98:
if (result instanceof HasSourceInfo) {
instanceof HasSourceInfoSetter?

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/Element.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Element.java:31: Element
getEnclosingElement();
I don't understand this change.  The EnclosingElement says something about the
scope.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Element.java:36: SourceInfo
getNameLocation();
We will need to be careful about persisting this field.  There is that static
weak hash map we must make sure is repopulated.

[zundel, 2012-03-12 20:05:57]

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File
compiler/java/com/google/dart/compiler/resolver/FieldElementImplementation.java
(right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/FieldElementImplementation.java:12:
import com.google.dart.compiler.common.SourceInfo;
If its just a couple of files, I don't mind but in the future, try not to mix
auto-formatter changes with actual code changes.  Usually what I try to do to
make it easier on reviewers is to just run through the files in the CL and
format them in a separate, standalone CL that can get an easy lgtm  before
uploading the rest of the changes.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/MemberBuilder.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/MemberBuilder.java:62: private
Element enclosingElement;
This should be EnclosingElement just as a way of enforcing type safety. There
are only a few types that are valid enclosing elements for fields and method
definitions.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:395: //String
defaultClassSource = getTypeDraftSourceString(defaultClassType);
remove commented out code?

[scheglov, 2012-03-12 20:24:58]

Eric, thank you for comments.
I will integrate them into the next CL.

I know that it is not good to format and make changes, but sometimes this
happens automatically in the middle of long debugging/fixing process. I actually
had no intension to format them, just missed this :-(

[scheglov, 2012-03-13 00:42:25]

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/LibraryDepsVisitor.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/LibraryDepsVisitor.java:48: Element
enclosing = target.getEnclosingElement();
On 2012/03/12 19:42:31, zundel wrote:
> Why the removal of EnclosingElement everywhere?  You have removed many
> references to it but it still exists in the class hierarchy.  Part of the JDK6
> bug?

Because I've tried to make "enclosingElement" just "parentElement".
For example for VariableElement the MethodElement is "enclosing", but are we
going to track all local variables which exist in method and return them from
getMembers()?

Hm...
Looks that you are right, we can make getEnclosingElement() always return
EnclosingElement.
I will fix this.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/parser/DartScannerParserContext.java
(right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/parser/DartScannerParserContext.java:98:
if (result instanceof HasSourceInfo) {
On 2012/03/12 19:42:31, zundel wrote:
> instanceof HasSourceInfoSetter?

Done.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/Element.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Element.java:31: Element
getEnclosingElement();
On 2012/03/12 19:42:31, zundel wrote:
> I don't understand this change.  The EnclosingElement says something about the
> scope.

I will undo this in the next CL.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Element.java:36: SourceInfo
getNameLocation();
On 2012/03/12 19:42:31, zundel wrote:
> We will need to be careful about persisting this field.  There is that static
> weak hash map we must make sure is repopulated.

This map is populated lazily.
Basically we will serialize SourceInfo as resource identifier, offset and
length.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/Elements.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Elements.java:444: public static
SourceInfo getNameLocation(Element element) {
On 2012/03/12 19:09:26, Brian Wilkerson wrote:
> In a subsequent CL this method should go away.

Done.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/MemberBuilder.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/MemberBuilder.java:62: private
Element enclosingElement;
On 2012/03/12 20:05:57, zundel wrote:
> This should be EnclosingElement just as a way of enforcing type safety. There
> are only a few types that are valid enclosing elements for fields and method
> definitions.

Done.

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

https://chromiumcodereview.appspot.com/9692002/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:395: //String
defaultClassSource = getTypeDraftSourceString(defaultClassType);
On 2012/03/12 20:05:57, zundel wrote:
> remove commented out code?

Done.