code review 5643066: go/printer: implement SourcePos mode

go/printer: implement SourcePos mode

If a printer is configured with the SourcePos mode
set, it will emit //-line comments as necessary to
ensure that the result - if reparsed - reflects the
original source position information.

This change required a bit of reworking of the
output section in printer.go. Specifically:

- Introduced new Config mode 'SourcePos'.

- Introduced new position 'out' which tracks the
position of the generated output if it were read
in again. If there is a discrepancy between out
and the current AST/source position, a //line
comment is emitted to correct for it.

- Lazy emission of indentation so that //line
comments can be placed correctly. As a result,
the trimmer will have to do less work.

- Merged writeItem into writeString.

- Merged writeByteN into writeByte.

- Use a []byte instead of a byte.Buffer both in the
printer and in the trimmer (eliminates dependency).

Also: introduced explicit printer.Mode type (in
sync w/ parser.Mode, scanner.Mode, etc.)

Runs all tests. Applied gofmt to src, misc w/o changes.

Fixes issue 1047.
Fixes issue 2697.

[gri, 2012-02-09 22:57:18]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[rsc1, 2012-02-10 20:12:07]

LGTM

Thanks very much.

http://codereview.appspot.com/5643066/diff/9006/src/pkg/go/printer/printer.go
File src/pkg/go/printer/printer.go (right):

http://codereview.appspot.com/5643066/diff/9006/src/pkg/go/printer/printer.go...
src/pkg/go/printer/printer.go:61: // The out position differs from the pos
position when then result
s/then/the/

[gri, 2012-02-10 21:27:38]

*** Submitted as 507582df4ca6 ***

go/printer: implement SourcePos mode

If a printer is configured with the SourcePos mode
set, it will emit //-line comments as necessary to
ensure that the result - if reparsed - reflects the
original source position information.

This change required a bit of reworking of the
output section in printer.go. Specifically:

- Introduced new Config mode 'SourcePos'.

- Introduced new position 'out' which tracks the
position of the generated output if it were read
in again. If there is a discrepancy between out
and the current AST/source position, a //line
comment is emitted to correct for it.

- Lazy emission of indentation so that //line
comments can be placed correctly. As a result,
the trimmer will have to do less work.

- Merged writeItem into writeString.

- Merged writeByteN into writeByte.

- Use a []byte instead of a byte.Buffer both in the
printer and in the trimmer (eliminates dependency).

Also: introduced explicit printer.Mode type (in
sync w/ parser.Mode, scanner.Mode, etc.)

Runs all tests. Applied gofmt to src, misc w/o changes.

Fixes issue 1047.
Fixes issue 2697.

R=rsc, rsc
CC=golang-dev
http://codereview.appspot.com/5643066

[mc, 2012-02-10 23:32:41]

Apologize if this was targeting another line number issues. I am
currently trying (my older go project for kids) to get go-gtk working with the
current go tip
( https://code.google.com/p/go/issues/detail?id=2976 )
and I can't see any improvement after this commit!?!?

I've got error messages:

mygo  build -s
# github.com/mattn/go-gtk/gtk
gtk.go: In function ‘_cgo_180a90a506dc_Cfunc_gtk_about_dialog_set_documenters’:
gtk.go:3158:2: warning: passing argument 2 of ‘gtk_about_dialog_set_documenters’
from incompatible pointer type [enabled by default]
/usr/include/gtk-2.0/gtk/gtkaboutdialog.h:114:24: note: expected ‘const gchar
**’ but argument is of type ‘gchar **’
gtk.go: In function ‘_cgo_180a90a506dc_Cfunc_gtk_about_dialog_set_authors’:
gtk.go:7681:2: warning: passing argument 2 of ‘gtk_about_dialog_set_authors’
from incompatible pointer type [enabled by default]
/usr/include/gtk-2.0/gtk/gtkaboutdialog.h:111:24: note: expected ‘const gchar
**’ but argument is of type ‘gchar **’
gtk.go: In function ‘_cgo_180a90a506dc_Cfunc_gtk_about_dialog_set_artists’:
gtk.go:9401:2: warning: passing argument 2 of ‘gtk_about_dialog_set_artists’
from incompatible pointer type [enabled by default]
/usr/include/gtk-2.0/gtk/gtkaboutdialog.h:117:24: note: expected ‘const gchar
**’ but argument is of type ‘gchar **’
# github.com/mattn/go-gtk/gtk
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkProgressBar._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector _Ctype_GtkProgressBar._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
cannot use &p._ (type *[4]byte) as type *[8]byte in function argument
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkProgressBar._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector _Ctype_GtkProgressBar._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkProgressBar._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector _Ctype_GtkProgressBar._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkFrame._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector _Ctype_GtkFrame._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkFrame._
/tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1: too
many errors



_cgo_gotypes.go is filled with //line :1 tokens:

 type _Ctype_struct__GtkStatusbar struct {
//line :1
	parent_widget	_Ctype_GtkHBox
//line :1
	frame		*_Ctype_GtkWidget
//line :1
	label		*_Ctype_GtkWidget
...



and e.g. gtk.go:3158 seems to have nothing to do with
"gtk_about_dialog_set_documenters":

func (v *GtkTreeModel) GetFlags() GtkTreeModelFlags {
	return GtkTreeModelFlags(C.gtk_tree_model_get_flags(v.TreeModel))
}

What am I missing or doing wrong?

Thanks,
Martin

[gri, 2012-02-10 23:50:31]

The changes to the cgo code may not have been correct as there appears
to be no functional test for cgo at the moment. The go/printer does
produce the correct //line information but cgo may not use the correct
go/printer configuration in all cases.

If you could provide a simple stand-alone test case that is broken,
that would be excellent. Please file an issue with it.

- gri

On Fri, Feb 10, 2012 at 3:32 PM,  <untheoretic@googlemail.com> wrote:
> Apologize if this was targeting another line number issues. I am
> currently trying (my older go project for kids) to get go-gtk working with the
> current go tip
> ( https://code.google.com/p/go/issues/detail?id=2976 )
> and I can't see any improvement after this commit!?!?
>
> I've got error messages:
>
> mygo  build -s
> # github.com/mattn/go-gtk/gtk
> gtk.go: In function
‘_cgo_180a90a506dc_Cfunc_gtk_about_dialog_set_documenters’:
> gtk.go:3158:2: warning: passing argument 2 of
‘gtk_about_dialog_set_documenters’ from incompatible pointer type [enabled by
default]
> /usr/include/gtk-2.0/gtk/gtkaboutdialog.h:114:24: note: expected ‘const gchar
**’ but argument is of type ‘gchar **’
> gtk.go: In function ‘_cgo_180a90a506dc_Cfunc_gtk_about_dialog_set_authors’:
> gtk.go:7681:2: warning: passing argument 2 of ‘gtk_about_dialog_set_authors’
from incompatible pointer type [enabled by default]
> /usr/include/gtk-2.0/gtk/gtkaboutdialog.h:111:24: note: expected ‘const gchar
**’ but argument is of type ‘gchar **’
> gtk.go: In function ‘_cgo_180a90a506dc_Cfunc_gtk_about_dialog_set_artists’:
> gtk.go:9401:2: warning: passing argument 2 of ‘gtk_about_dialog_set_artists’
from incompatible pointer type [enabled by default]
> /usr/include/gtk-2.0/gtk/gtkaboutdialog.h:117:24: note: expected ‘const gchar
**’ but argument is of type ‘gchar **’
> # github.com/mattn/go-gtk/gtk
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkProgressBar._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector _Ctype_GtkProgressBar._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
cannot use &p._ (type *[4]byte) as type *[8]byte in function argument
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkProgressBar._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector _Ctype_GtkProgressBar._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkProgressBar._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector _Ctype_GtkProgressBar._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkFrame._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector _Ctype_GtkFrame._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1:
ambiguous selector *_Ctype_GtkFrame._
> /tmp/go-build434705490/github.com/mattn/go-gtk/gtk/_obj/_cgo_gotypes.go:1: too
many errors
>
>
>
> _cgo_gotypes.go is filled with //line :1 tokens:
>
>  type _Ctype_struct__GtkStatusbar struct {
> //line :1
>        parent_widget   _Ctype_GtkHBox
> //line :1
>        frame           *_Ctype_GtkWidget
> //line :1
>        label           *_Ctype_GtkWidget
> ...
>
>
>
> and e.g. gtk.go:3158 seems to have nothing to do with
"gtk_about_dialog_set_documenters":
>
> func (v *GtkTreeModel) GetFlags() GtkTreeModelFlags {
>        return GtkTreeModelFlags(C.gtk_tree_model_get_flags(v.TreeModel))
> }
>
> What am I missing or doing wrong?
>
> Thanks,
> Martin