Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(525)

Issue 7351044: code review 7351044: cmd/cgo, cmd/dist, cmd/go: cgo with clang fixes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by minux1
Modified:
11 years, 1 month ago
Reviewers:
rsc
CC:
golang-dev, dfc, iant
Visibility:
Public.

Description

cmd/cgo, cmd/dist, cmd/go: cgo with clang fixes 1. Workaround the smart clang diagnostics with -Qunused-arguments: clang: error: argument unused during compilation: '-XXX' 2. if "clang -print-libgcc-file-name" returns non-absolute path, don't provide that on linker command line. 3. Fix dwarf.PtrType.Size() in cmd/cgo as clang doesn't generate DW_AT_byte_size for pointer types. 4. Workaround warnings for -Wno-unneeded-internal-declaration with -Wno-unknown-warning-option. 5. Add -Wno-unused-function. 6. enable race detector test on darwin with clang (at least Apple clang version 1.7 (tags/Apple/clang-77) works). Requires CL 7354043. Update issue 4829 This should fix most parts of the problem, but one glitch still remains. DWARF generated by newer clang doesn't differentiate these two function types: void *malloc(size_t); void *malloc(unsigned long int); so you might need to do this to make make.bash pass: sed -i -e 's/C.malloc(C.size_t/C.malloc(C.ulong/' pkg/os/user/lookup_unix.go

Patch Set 1 #

Patch Set 2 : diff -r 0adf91947752 https://code.google.com/p/go/ #

Patch Set 3 : diff -r 6f23480a76e1 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 6f23480a76e1 https://code.google.com/p/go/ #

Patch Set 5 : diff -r 6f23480a76e1 https://code.google.com/p/go/ #

Patch Set 6 : diff -r 6f23480a76e1 https://code.google.com/p/go/ #

Patch Set 7 : diff -r 6f23480a76e1 https://code.google.com/p/go/ #

Patch Set 8 : diff -r 99c8e7dc4199 https://code.google.com/p/go/ #

Patch Set 9 : diff -r dd18b993ba95 https://code.google.com/p/go/ #

Total comments: 3

Patch Set 10 : diff -r 526fb7125a2f https://code.google.com/p/go/ #

Patch Set 11 : diff -r 526fb7125a2f https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -22 lines) Patch
M src/cmd/cgo/gcc.go View 1 2 3 4 5 6 2 chunks +12 lines, -0 lines 0 comments Download
M src/cmd/dist/build.c View 1 2 3 4 5 6 7 8 9 2 chunks +5 lines, -4 lines 0 comments Download
M src/cmd/go/build.go View 1 2 3 4 5 6 7 8 9 6 chunks +24 lines, -14 lines 0 comments Download
M src/run.bash View 1 2 3 4 5 1 chunk +2 lines, -4 lines 0 comments Download

Messages

Total messages: 14
dfc
Please hg mail this for review.
11 years, 1 month ago (2013-02-19 01:41:06 UTC) #1
dfc
On 2013/02/19 01:41:06, dfc wrote: > Please hg mail this for review. This did not ...
11 years, 1 month ago (2013-02-19 02:08:05 UTC) #2
minux1
On Tuesday, February 19, 2013, wrote: > On 2013/02/19 01:41:06, dfc wrote: > >> Please ...
11 years, 1 month ago (2013-02-19 05:51:49 UTC) #3
dfc
Ahh, my bad, i'll wait for those CLs to be mailed On 19/02/2013, at 16:51, ...
11 years, 1 month ago (2013-02-19 06:16:00 UTC) #4
minux1
For the record, the way to test this CL is: cd $GOROOT/src # assuming it's ...
11 years, 1 month ago (2013-02-19 07:20:29 UTC) #5
iant
LGTM I suppose.
11 years, 1 month ago (2013-02-20 02:41:16 UTC) #6
minux1
cmd/cgo, cmd/dist, cmd/go: cgo with clang fixes 1. Workaround the smart clang diagnostics with -Qunused-arguments: ...
11 years, 1 month ago (2013-02-20 16:15:22 UTC) #7
minux1
Hello golang-dev@googlegroups.com, dave@cheney.net, iant@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
11 years, 1 month ago (2013-02-20 16:21:21 UTC) #8
rsc
LGTM https://codereview.appspot.com/7351044/diff/29001/src/cmd/dist/build.c File src/cmd/dist/build.c (right): https://codereview.appspot.com/7351044/diff/29001/src/cmd/dist/build.c#newcode412 src/cmd/dist/build.c:412: "-ggdb", Not sure why this is being added. ...
11 years, 1 month ago (2013-02-22 22:36:48 UTC) #9
minux1
*** Submitted as https://code.google.com/p/go/source/detail?r=04d884e0f760 *** cmd/cgo, cmd/dist, cmd/go: cgo with clang fixes 1. Workaround the ...
11 years, 1 month ago (2013-02-23 12:25:04 UTC) #10
rsc
Sorry, but the copy of clang I have rejects -ggdb. That's why I removed it. ...
11 years, 1 month ago (2013-02-25 21:49:23 UTC) #11
minux1
On Tue, Feb 26, 2013 at 5:49 AM, <rsc@golang.org> wrote: > Sorry, but the copy ...
11 years, 1 month ago (2013-02-25 21:51:02 UTC) #12
rsc
On Mon, Feb 25, 2013 at 4:50 PM, minux <minux.ma@gmail.com> wrote: > what version is ...
11 years, 1 month ago (2013-02-25 22:19:45 UTC) #13
dfc
11 years, 1 month ago (2013-02-25 22:52:22 UTC) #14
If you use macports, sudo port install clang-3.2 works well, also
http://llvm.org/releases/3.2/clang+llvm-3.2-x86_64-apple-darwin11.tar.gz
is another option.

On Tue, Feb 26, 2013 at 9:19 AM, Russ Cox <rsc@golang.org> wrote:
> On Mon, Feb 25, 2013 at 4:50 PM, minux <minux.ma@gmail.com> wrote:
>>
>> what version is it?
>
>
> I don't know. Someone here who works on clang gave me a binary. It's my
> problem, don't worry about it. :-)
> I'm glad it's working with a clang other people actually have.
>
> Russ
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b