code review 135990043: misc/cgo/test: disable issue 8428 regress test on darwin

misc/cgo/test: disable issue 8428 regress test on darwin

Clang 3.2 and older (as shipped with OS X Mountain Lion and older)
outputs ambiguous DWARF debug info that makes it impossible for us to
reconstruct accurate type information as required for this test.

Fixes issue 8611.

[mdempsky, 2014-08-29 00:25:35]

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

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

[minux, 2014-08-29 00:36:28]

https://codereview.appspot.com/135990043/diff/40001/misc/cgo/test/issue8428mo...
File misc/cgo/test/issue8428more.go (right):

https://codereview.appspot.com/135990043/diff/40001/misc/cgo/test/issue8428mo...
misc/cgo/test/issue8428more.go:7: // +build !darwin
this build tag won't take effect.

[mdempsky, 2014-08-29 01:33:05]

https://codereview.appspot.com/135990043/diff/40001/misc/cgo/test/issue8428mo...
File misc/cgo/test/issue8428more.go (right):

https://codereview.appspot.com/135990043/diff/40001/misc/cgo/test/issue8428mo...
misc/cgo/test/issue8428more.go:7: // +build !darwin
On 2014/08/29 00:36:27, minux wrote:
> this build tag won't take effect.

Do you mind elaborating why you say that and/or what you think needs to be done
differently?

In particular, it seems to work as expected if I use the same header in a
standalone program.

It also seems to meet the requirements described at
http://golang.org/pkg/go/build/: "Constraints may appear in any kind of source
file (not just Go), but they must appear near the top of the file, preceded only
by blank lines and other line comments. [...] To distinguish build constraints
from package documentation, a series of build constraints must be followed by a
blank line."

[rsc, 2014-08-29 02:23:12]

The errors I sent on the old CL included:

./issue8428.go:40: cannot use [0]C.char literal (type [0]C.char) as
type [1]C.char in field value

That line is not moving in this CL. It seems like it needs to. I would just
revert the change as written and add the build tag to issue8428.go.

[mdempsky, 2014-08-29 02:27:32]

On 2014/08/29 02:23:12, rsc wrote:
> The errors I sent on the old CL included:
> 
> ./issue8428.go:40: cannot use [0]C.char literal (type [0]C.char) as
> type [1]C.char in field value
> 
> That line is not moving in this CL. It seems like it needs to.

Oops, right, I missed that line.

> I would just
> revert the change as written and add the build tag to issue8428.go.

Done.

[rsc, 2014-08-29 02:31:18]

LGTM

[minux, 2014-08-29 02:32:03]

On Thu, Aug 28, 2014 at 9:33 PM,  <mdempsky@google.com> wrote:
>
https://codereview.appspot.com/135990043/diff/40001/misc/cgo/test/issue8428mo...
> misc/cgo/test/issue8428more.go:7: // +build !darwin
> On 2014/08/29 00:36:27, minux wrote:
>>
>> this build tag won't take effect.
> Do you mind elaborating why you say that and/or what you think needs to
> be done differently?
You're right. My understanding of build tags are flawed.
I always put them with blank lines on both sides, but
it seems it doesn't have to be that way (only blank lines
at the end is required.)

[rsc, 2014-08-29 02:40:35]

On Thu, Aug 28, 2014 at 10:31 PM, minux <minux@golang.org> wrote:

> On Thu, Aug 28, 2014 at 9:33 PM,  <mdempsky@google.com> wrote:
> >
>
https://codereview.appspot.com/135990043/diff/40001/misc/cgo/test/issue8428mo...
> > misc/cgo/test/issue8428more.go:7: // +build !darwin
> > On 2014/08/29 00:36:27, minux wrote:
> >>
> >> this build tag won't take effect.
> > Do you mind elaborating why you say that and/or what you think needs to
> > be done differently?
> You're right. My understanding of build tags are flawed.
> I always put them with blank lines on both sides, but
> it seems it doesn't have to be that way (only blank lines
> at the end is required.)
>

you don't even need a blank line immediately after.
you just need a blank line before the package statement so that it is not a
package doc.

[rsc, 2014-08-29 02:41:00]

*** Submitted as https://code.google.com/p/go/source/detail?r=7d8ecaa0c2eb ***

misc/cgo/test: disable issue 8428 regress test on darwin

Clang 3.2 and older (as shipped with OS X Mountain Lion and older)
outputs ambiguous DWARF debug info that makes it impossible for us to
reconstruct accurate type information as required for this test.

Fixes issue 8611.

LGTM=rsc
R=r, rsc, minux
CC=golang-codereviews
https://codereview.appspot.com/135990043

Committer: Russ Cox <rsc@golang.org>

[minux, 2014-08-29 02:43:21]

On Thu, Aug 28, 2014 at 10:40 PM, Russ Cox <rsc@golang.org> wrote:
> On Thu, Aug 28, 2014 at 10:31 PM, minux <minux@golang.org> wrote:
>> You're right. My understanding of build tags are flawed.
>> I always put them with blank lines on both sides, but
>> it seems it doesn't have to be that way (only blank lines
>> at the end is required.)
> you don't even need a blank line immediately after.
> you just need a blank line before the package statement so that it is not a
> package doc.
i see. Thanks. Basically it's just comment. at long as it's not part of the
package docs, it will take effect.