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

Issue 20060044: code review 20060044: [release-branch.go1.2] cmd/cgo: stop using compiler err... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 5 months ago by adg
Modified:
10 years, 5 months ago
Reviewers:
dsymonds
CC:
golang-dev
Visibility:
Public.

Description

[release-branch.go1.2] cmd/cgo: stop using compiler error message text to analyze C names ««« CL 15070043 / 90a628ac54ed cmd/cgo: stop using compiler error message text to analyze C names The old approach to determining whether "name" was a type, constant, or expression was to compile the C program name; and scan the errors and warnings generated by the compiler. This requires looking for specific substrings in the errors and warnings, which ties the implementation to specific compiler versions. As compilers change their errors or drop warnings, cgo breaks. This happens slowly but it does happen. Clang in particular (now required on OS X) has a significant churn rate. The new approach compiles a slightly more complex program that is either valid C or not valid C depending on what kind of thing "name" is. It uses only the presence or absence of an error message on a particular line, not the error text itself. The program is: // error if and only if name is undeclared void f1(void) { typeof(name) *x; } // error if and only if name is not a type void f2(void) { name *x; } // error if and only if name is not an integer constant void f3(void) { enum { x = (name)*1 }; } I had not been planning to do this until Go 1.3, because it is a non-trivial change, but it fixes a real Xcode 5 problem in Go 1.2, and the new code is easier to understand than the old code. It should be significantly more robust. Fixes issue 6596. Fixes issue 6612. R=golang-dev, r, james, iant CC=golang-dev https://codereview.appspot.com/15070043 »»»

Patch Set 1 #

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+229 lines, -149 lines) Patch
M misc/cgo/test/cgo_test.go View 1 1 chunk +1 line, -0 lines 0 comments Download
A misc/cgo/test/issue6612.go View 1 1 chunk +93 lines, -0 lines 0 comments Download
M src/cmd/cgo/doc.go View 1 1 chunk +21 lines, -20 lines 0 comments Download
M src/cmd/cgo/gcc.go View 1 4 chunks +114 lines, -129 lines 0 comments Download

Messages

Total messages: 3
adg
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
10 years, 5 months ago (2013-11-01 00:04:10 UTC) #1
adg
*** Submitted as https://code.google.com/p/go/source/detail?r=2acdaa9fe432 *** [release-branch.go1.2] cmd/cgo: stop using compiler error message text to analyze ...
10 years, 5 months ago (2013-11-01 00:05:27 UTC) #2
dsymonds
10 years, 5 months ago (2013-11-01 00:06:11 UTC) #3
LGTM
Sign in to reply to this message.

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