This is an archive of the discontinued LLVM Phabricator instance.

[llgo] Introduce llgoi, a REPL for Go
ClosedPublic

Authored by pcc on Jan 13 2015, 12:50 PM.

Details

Summary

llgoi is a Go REPL based on llgo irgen and the LLVM JIT. It supports
expressions, statements, most declarations and imports, including binary
imports from the standard library and source imports from $GOPATH.

Diff Detail

Event Timeline

pcc updated this revision to Diff 18109.Jan 13 2015, 12:50 PM
pcc retitled this revision from to [llgo] Introduce llgoi, a REPL for Go.
pcc updated this object.
pcc edited the test plan for this revision. (Show Details)
pcc added a reviewer: axw.
pcc added a subscriber: Unknown Object (MLST).
axw edited edge metadata.Jan 13 2015, 9:20 PM

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.

cmd/llgoi/llgoi.go
96

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.

126

lowercase, for consistency

130

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.

282

fmt.Fprintf(&code, "package %s", pkgname)

288

fmt.Fprintf(&code, "import %q\n", pkg.Path())

312

Should this error if len(assigns)>0 and there are no new variables?

327

fmt.Fprintf(&code, " = %s\n\n", l.line)

356

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)

476

Should we be making in.imports a map, to better handle duplicate imports? If you do that now, llgoi panics.

509

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 ...

pcc updated this revision to Diff 18141.Jan 13 2015, 11:50 PM
pcc edited edge metadata.

Address comments

pcc added a comment.Jan 13 2015, 11:50 PM

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.

I agree that the diagnostics could be improved in many cases. Ideally I would like llgoi to be able to do all parsing/type checking of the user input up front, and treat any errors from parsing/type checking the final package as bugs, but I'm not entirely sure how best this should be achieved.

cmd/llgoi/llgoi.go
96

Ah, yes, fixed. It seemed neater for loadSourcePackage to take a copts instead.

126

Done.

130

Done.

282

Done.

288

Done.

312

Yes it should; I felt that this was more a QOI issue so I left it out of the initial version.

327

Done

356

Done.

Yes, the whitespace is semantically insignificant (except for semicolon insertion at newlines), but it is useful if we want to look at the source.

476

We should be handling a number of things of that nature better. As one other example, if you declare a variable with the same name as an import, llgoi gets confused and refuses to interpret any more lines. I figured this would be good enough for the initial version though.

509

I guess eventually we'd want to use GNU readline, or golang.org/x/crypto/ssh/terminal, or ...

Yes, or something like that. I think it would be very cool if this could do tab completion, but I couldn't find a suitable package (not to mention the details of actually computing the completions, which doesn't seem easy with the current go/types package).

axw accepted this revision.Jan 13 2015, 11:56 PM
axw edited edge metadata.

LGTM.

Will you be announcing this on golang-nuts? I think you'll make a bunch of people happy, and I can easily see it attracting more help.

This revision is now accepted and ready to land.Jan 13 2015, 11:56 PM
pcc added a comment.Jan 14 2015, 8:14 PM

I'll announce it soon. I would like an upstream change to land first, specifically https://go-review.googlesource.com/#/c/2137/ , as it fixes certain bugs that occur when importing source packages.

This revision was automatically updated to reflect the committed changes.