Split AST Declaration class, in preparation for new module declaration forms.

Split AST Declaration class, in preparation for new module declaration forms.

Turns Declaration into an abstract class, and introduces VariableDeclaration as a concrete subclass.

R=kmillikin@chromium.org
BUG=
TEST=

Committed: https://code.google.com/p/v8/source/detail?r=10662

[Kevin Millikin (Chromium), 2012-02-08 15:23:03]

OK, LGTM.

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode444
src/ast.h:444: virtual bool IsInlineable() const;
I think this method is obsolete (and the check in hydrogen.cc is unnecessary),
because the declaration's inline-ability is already recorded in the function
containing it.  Talk to jkummerow.

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode473
src/ast.h:473: class VariableDeclaration: public Declaration {
It's a little weird that VariableDeclaration includes 'function' and 'const' as
well as 'var' and 'let'.  Does the (proposed) ES grammar have a name for those
things?

[Jakob Kummerow, 2012-02-08 15:40:28]

IsInlineable() LGTM.

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode444
src/ast.h:444: virtual bool IsInlineable() const;
On 2012/02/08 15:23:04, Kevin Millikin wrote:
> I think this method is obsolete (and the check in hydrogen.cc is unnecessary),
> because the declaration's inline-ability is already recorded in the function
> containing it.  Talk to jkummerow.

This is true for all AstNodes *except* Declarations, because inline-ability of
declarations is not known yet when the AstNodeFactory creates them.

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode450
src/ast.h:450: template<class> friend class AstNodeFactory;
I think you don't need this any more.

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode475
src/ast.h:475: VariableDeclaration(VariableProxy* proxy,
Suggestion: Move the constructor to a "protected:" section and add a
"template<class> friend class AstNodeFactory;" in that section too, then the
compiler will enforce for you that nobody accidentally calls the c'tor directly.

[rossberg, 2012-02-08 15:46:43]

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode444
src/ast.h:444: virtual bool IsInlineable() const;
On 2012/02/08 15:23:04, Kevin Millikin wrote:
> I think this method is obsolete (and the check in hydrogen.cc is unnecessary),
> because the declaration's inline-ability is already recorded in the function
> containing it.  Talk to jkummerow.

I can't tell, but according to Jakob, the info is available for all AstNodes,
except Declarations, because they are kind of special, e.g. because they live in
the scope.

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode473
src/ast.h:473: class VariableDeclaration: public Declaration {
On 2012/02/08 15:23:04, Kevin Millikin wrote:
> It's a little weird that VariableDeclaration includes 'function' and 'const'
as
> well as 'var' and 'let'.  Does the (proposed) ES grammar have a name for those
> things?

Not yet, it currently still just calls them "declarations" collectively, because
modules haven't made it into the draft yet. The grammar in the modules proposal
uses VariableDeclaration to include 'const' (it seems), but not 'function'.

Would LocalDeclaration be better? (Those are the ones that are allowed locally,
although they may also appear in global or module scope.)

[rossberg, 2012-02-08 15:52:21]

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode450
src/ast.h:450: template<class> friend class AstNodeFactory;
On 2012/02/08 15:40:28, Jakob wrote:
> I think you don't need this any more.

Done.

https://chromiumcodereview.appspot.com/9348057/diff/1/src/ast.h#newcode475
src/ast.h:475: VariableDeclaration(VariableProxy* proxy,
On 2012/02/08 15:40:28, Jakob wrote:
> Suggestion: Move the constructor to a "protected:" section and add a
> "template<class> friend class AstNodeFactory;" in that section too, then the
> compiler will enforce for you that nobody accidentally calls the c'tor
directly.

Done.