Issue 9373023: Extend AST with basic module constructs (yet unused).

Issue 9373023: Extend AST with basic module constructs (yet unused). (Closed)

Created:
8 years, 10 months ago by rossberg

Modified:
8 years, 10 months ago

Reviewers:
Jakob Kummerow

CC:
v8-dev

Base URL:
https://v8.googlecode.com/svn/branches/bleeding_edge

Visibility:
Public.

More Reviews

Description

Extend AST with basic module constructs (yet unused). R=jkummerow@chromium.org BUG= TEST= Committed: https://code.google.com/p/v8/source/detail?r=10663

Patch Set 2 : Fixed baseline. #

Total comments: 8

Patch Set 3 : Addresses Jakob's comments. #

Created: 8 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+287 lines, -9 lines)			Patch
M	src/ast.h	View	1	7 chunks	+144 lines, -4 lines	0 comments	Download
M	src/ast.cc	View	1 2	1 chunk	+5 lines, -0 lines	0 comments	Download
M	src/full-codegen.cc	View	1	3 chunks	+47 lines, -5 lines	0 comments	Download
M	src/hydrogen.cc	View	1 2	1 chunk	+25 lines, -0 lines	0 comments	Download
M	src/prettyprinter.cc	View	1 2	2 chunks	+61 lines, -0 lines	0 comments	Download
M	src/rewriter.cc	View	1	1 chunk	+5 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Jakob Kummerow

LGTM with comments. https://chromiumcodereview.appspot.com/9373023/diff/2001/src/ast.h File src/ast.h (right): https://chromiumcodereview.appspot.com/9373023/diff/2001/src/ast.h#newcode2520 src/ast.h:2520: VISIT_AND_RETURN(ModuleDeclaration, decl) You'll want to implement ...

8 years, 10 months ago (2012-02-09 13:18:35 UTC) #2

rossberg

8 years, 10 months ago (2012-02-09 13:26:31 UTC) #3

https://chromiumcodereview.appspot.com/9373023/diff/2001/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/9373023/diff/2001/src/ast.h#newcode2520
src/ast.h:2520: VISIT_AND_RETURN(ModuleDeclaration, decl)
On 2012/02/09 13:18:35, Jakob wrote:
> You'll want to implement the matching Visit____ methods in
> AstConstructionVisitor to make this work.

Done.

https://chromiumcodereview.appspot.com/9373023/diff/2001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/9373023/diff/2001/src/hydrogen.cc#newc...
src/hydrogen.cc:6593: 
On 2012/02/09 13:18:35, Jakob wrote:
> nit: we generally have two empty lines between top-level things.

Done.

https://chromiumcodereview.appspot.com/9373023/diff/2001/src/prettyprinter.cc
File src/prettyprinter.cc (right):

https://chromiumcodereview.appspot.com/9373023/diff/2001/src/prettyprinter.cc...
src/prettyprinter.cc:84: 
On 2012/02/09 13:18:35, Jakob wrote:
> nit: two empty lines

Done.

https://chromiumcodereview.appspot.com/9373023/diff/2001/src/prettyprinter.cc...
src/prettyprinter.cc:770: 
On 2012/02/09 13:18:35, Jakob wrote:
> same here

Done.

Expand Messages | Collapse Messages