code review 6497098: gc: initial BOM is legal.

gc: initial BOM is legal.
Fixes issue 4040.

[r, 2012-09-07 21:18:32]

Hello golang-dev@googlegroups.com,

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

[rsc, 2012-09-10 16:25:52]

http://codereview.appspot.com/6497098/diff/1003/src/cmd/gc/lex.c
File src/cmd/gc/lex.c (right):

http://codereview.appspot.com/6497098/diff/1003/src/cmd/gc/lex.c#newcode323
src/cmd/gc/lex.c:323: c = getr();
I'm worried about line number manipulation if c == '\n'. This code does look
right, but maybe we can just use bufio and avoid the issue entirely:

// Skip initial BOM if present.
if(Bgetrune(curio.bin) != 0xFEFF)
    Bungetrune();

http://codereview.appspot.com/6497098/diff/1003/src/cmd/gc/lex.c#newcode776
src/cmd/gc/lex.c:776: return 0;
if(c == 0xfeff)
    return 1;

(This will give a good diagnostic when a BOM shows up outside a string or
comment. Or if you want to work byte-order mark into the message, add a check
near the current call to isfrog.)

http://codereview.appspot.com/6497098/diff/1003/test/syntax/bom.go
File test/syntax/bom.go (right):

http://codereview.appspot.com/6497098/diff/1003/test/syntax/bom.go#newcode7
test/syntax/bom.go:7: // This file starts with a BOM that is ignored.
Probably better to write a 'runout' test like test/crlf.go.

[r, 2012-09-10 18:32:47]

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

Please take another look.

[r, 2012-09-10 18:34:41]

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

Please take another look.

[r, 2012-09-10 18:34:59]

This is what I meant to do.

-rob

[rsc, 2012-09-10 19:45:13]

LGTM

http://codereview.appspot.com/6497098/diff/9002/src/cmd/gc/lex.c
File src/cmd/gc/lex.c (right):

http://codereview.appspot.com/6497098/diff/9002/src/cmd/gc/lex.c#newcode1594
src/cmd/gc/lex.c:1594: yyerror("BOM in middle of file");
suggest "Unicode BOM..." or "UTF-8 BOM...".
just because 'unicode bom' has better search results than 'bom'.

http://codereview.appspot.com/6497098/diff/9002/test/bombad.go
File test/bombad.go (right):

http://codereview.appspot.com/6497098/diff/9002/test/bombad.go#newcode7
test/bombad.go:7: // This file starts with a BOM that is ignored.
Delete this comment? I assume there's no BOM at start in this file.

[r, 2012-09-10 20:03:19]

*** Submitted as http://code.google.com/p/go/source/detail?r=cc2bca9c03ef ***

gc: initial BOM is legal.
Fixes issue 4040.

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