[PATCH] [llgo] Introduce llgoi, a REPL for Go

[Andrew Wilkins]

This is great, thank you!

I wonder if we should be leaning a little more on go/parser, so we can identify and error on labeled statements, for example. I don't think we need to worry about that just now, though.


================
Comment at: cmd/llgoi/llgoi.go:96
@@ +95,3 @@
+		GenerateDebug:  true,
+		PackageCreated: in.augmentPackageScope,
+	}
----------------
This should only apply to on-the-fly source packages, not ones on-disk. Perhaps add another flag to loadSourcePackage which determines whether or not to augment.

================
Comment at: cmd/llgoi/llgoi.go:126
@@ +125,3 @@
+
+func (in *interp) Dispose() {
+	in.engine.Dispose()
----------------
lowercase, for consistency

================
Comment at: cmd/llgoi/llgoi.go:130
@@ +129,3 @@
+
+func (in *interp) loadSourcePackageFromCode(pkgcode, pkgpath string) (pkg *types.Package, err error) {
+	fset := token.NewFileSet()
----------------
Can we please follow https://github.com/golang/go/wiki/CodeReviewComments#named-result-parameters, and not named results unless it's helpful documentation-wise? I'd prefer no naked returns either. "return nil, err" makes it immediately obvious what all the result values are.

================
Comment at: cmd/llgoi/llgoi.go:282
@@ +281,3 @@
+	var code bytes.Buffer
+	code.WriteString("package ")
+	code.WriteString(pkgname)
----------------
fmt.Fprintf(&code, "package %s", pkgname)

================
Comment at: cmd/llgoi/llgoi.go:288
@@ +287,3 @@
+	for _, pkg := range in.imports {
+		code.WriteString("import \"")
+		code.WriteString(pkg.Path())
----------------
fmt.Fprintf(&code, "import %q\n", pkg.Path())

================
Comment at: cmd/llgoi/llgoi.go:312
@@ +311,3 @@
+
+		code.WriteString("var ")
+		for i := range typs {
----------------
Should this error if len(assigns)>0 and there are no new variables?

================
Comment at: cmd/llgoi/llgoi.go:327
@@ +326,3 @@
+		}
+		code.WriteString(" = ")
+		code.WriteString(l.line)
----------------
fmt.Fprintf(&code, " = %s\n\n", l.line)

================
Comment at: cmd/llgoi/llgoi.go:356
@@ +355,3 @@
+
+		code.WriteString("func init() {\n\t")
+		code.WriteString(l.line)
----------------
fmt.Fprintf(&code, "func init() {\n\t%s}", l.line)
(is the whitespace doing anything useful? I guess if we want to log the program source)

================
Comment at: cmd/llgoi/llgoi.go:476
@@ +475,3 @@
+		}
+		in.imports = append(in.imports, pkg)
+		return nil
----------------
Should we be making in.imports a map, to better handle duplicate imports? If you do that now, llgoi panics.

================
Comment at: cmd/llgoi/llgoi.go:509
@@ +508,3 @@
+
+	r := bufio.NewReader(os.Stdin)
+	for {
----------------
This would be a little neater using bufio.Scanner. I guess eventually we'd want to use GNU readline, or golang.org/x/crypto/ssh/terminal, or ...

http://reviews.llvm.org/D6957

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/