Quim Muntal has uploaded this change for review.
cmd/go: cgo export header to be compatible with MSVC complex types
After CL 379474 has landed, the only remaining cgo export header
incompatibility with MSVC is the use of the _Complex macro,
which is not not supported in MSVC even when it is part of the ISO C99
standard (1).
Since MSVC 2015 (2), complex math are supported via _Fcomplex and
_Dcomplex, which are equivalent to float _Complex and double _Complex.
As MSVC and C complex types have the same memory layout, we should
be able to typedef GoComplex64 and GoComplex128 to the appropriate
type in MSVC.
It is important to note that this CL is not adding MSVC support to cgo.
C compilers should still be GCC-compatible.
This CL is about allowing to include, without further modifications,
a DLL export header generated by cgo, normally using Mingw-W64 compiler,
into a MSVC project. This was already possible if the export header
changes introduced in this CL were done outside cgo, either manually or
in a post-build script.
Fixes #36233
1: https://docs.microsoft.com/en-us/cpp/c-runtime-library/complex-math-support
2: https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?c-standard-library-features-1
Change-Id: Iad8f26984b115c728e3b73f3a8334ade7a11cfa1
---
M src/cmd/cgo/gcc.go
M src/cmd/cgo/out.go
2 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
index 997a830..32074ef 100644
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -2392,13 +2392,19 @@
}
case *dwarf.ComplexType:
+ // MSVC does not support the complex keyword.
+ // Use GoComplex64 and GoComplex128 instead,
+ // which are typedef-ed to a compatible type.
+ // See go.dev/issues/36233.
switch t.Size {
default:
fatalf("%s: unexpected: %d-byte complex type - %s", lineno(pos), t.Size, dtype)
case 8:
t.Go = c.complex64
+ t.C.Set("GoComplex64")
case 16:
t.Go = c.complex128
+ t.C.Set("GoComplex128")
}
if t.Align = t.Size / 2; t.Align >= c.ptrSize {
t.Align = c.ptrSize
diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go
index 8ead173..3bdbdca 100644
--- a/src/cmd/cgo/out.go
+++ b/src/cmd/cgo/out.go
@@ -1895,8 +1895,14 @@
typedef size_t GoUintptr;
typedef float GoFloat32;
typedef double GoFloat64;
+#ifdef _MSC_VER
+#include <complex.h>
+typedef _Fcomplex GoComplex64;
+typedef _Dcomplex GoComplex128;
+#else
typedef float _Complex GoComplex64;
typedef double _Complex GoComplex128;
+#endif
/*
static assertion to make sure the file is being used on architecture
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Quim Muntal uploaded patch set #2 to this change.
2 files changed, 47 insertions(+), 0 deletions(-)
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Ian Lance Taylor.
Quim Muntal uploaded patch set #3 to this change.
cmd/go: cgo export header to be compatible with MSVC complex types
After CL 379474 has landed, the only remaining cgo export header
incompatibility with MSVC is the use of the _Complex macro,
which is not supported in MSVC even if it is part of the ISO C99
Attention is currently required from: Tobias Klauser, Quim Muntal.
2 comments:
Patchset:
Does MSVC support "float complex" and "double complex" if <complex.h> is #include'd? If so that might be preferable to using an #ifdef and _Complex.
File src/cmd/cgo/gcc.go:
Patch Set #3, Line 2395: // MSVC does not support the complex keyword.
Technically there is no keyword complex. There is a keyword _Complex. The header file <complex.h> #define's complex as a macro.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Quim Muntal uploaded patch set #4 to this change.
cmd/go: cgo export header to be compatible with MSVC complex types
After CL 379474 has landed, the only remaining cgo export header
incompatibility with MSVC is the use of the _Complex macro,
which is not not supported in MSVC even when it is part of the ISO C99
standard (1).
Since MSVC 2015 (2), complex math are supported via _Fcomplex and
_Dcomplex, which are equivalent to float _Complex and double _Complex.
As MSVC and C complex types have the same memory layout, we should
be able to typedef GoComplex64 and GoComplex128 to the appropriate
type in MSVC.
It is important to note that this CL is not adding MSVC support to cgo.
C compilers should still be GCC-compatible.
This CL is about allowing to include, without further modifications,
a DLL export header generated by cgo, normally using Mingw-W64 compiler,
into a MSVC project. This was already possible if the export header
changes introduced in this CL were done outside cgo, either manually or
in a post-build script.
Fixes #36233
1: https://docs.microsoft.com/en-us/cpp/c-runtime-library/complex-math-support
2: https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?c-standard-library-features-1
Change-Id: Iad8f26984b115c728e3b73f3a8334ade7a11cfa1
---
M src/cmd/cgo/gcc.go
M src/cmd/cgo/out.go
2 files changed, 46 insertions(+), 0 deletions(-)
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Ian Lance Taylor.
2 comments:
Patchset:
It doesn't, unfortunately. Quote from the MSVC docs linked above:
The compiler doesn't directly support a complex or _Complex keyword, therefore the Microsoft implementation uses structure types to represent complex numbers.
MSVC does implement most of complex functions defined in <complex.h>, but it does not support standard arithmetic operators, as _FComplex and friends are just normal structs.
File src/cmd/cgo/gcc.go:
Patch Set #3, Line 2395: // MSVC does not support the complex keyword.
Technically there is no keyword complex. There is a keyword _Complex. The header file <complex. […]
Good catch!
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Patch set 4:Run-TryBot +1
Attention is currently required from: Tobias Klauser, Quim Muntal.
Quim Muntal uploaded patch set #6 to this change.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Quim Muntal uploaded patch set #7 to this change.
cmd/go: cgo export header to be compatible with MSVC complex types
After CL 379474 has landed, the only remaining cgo export header
incompatibility with MSVC is the use of the _Complex macro,
which is not not supported in MSVC even when it is part of the ISO C99
standard (1).
Since MSVC 2015 (2), complex math are supported via _Fcomplex and
_Dcomplex, which are equivalent to float _Complex and double _Complex.
As MSVC and C complex types have the same memory layout, we should
be able to typedef GoComplex64 and GoComplex128 to the appropriate
type in MSVC.
It is important to note that this CL is not adding MSVC support to cgo.
C compilers should still be GCC-compatible.
This CL is about allowing to include, without further modifications,
a DLL export header generated by cgo, normally using Mingw-W64 compiler,
into a MSVC project. This was already possible if the export header
changes introduced in this CL were done outside cgo, either manually or
in a post-build script.
Fixes #36233
1: https://docs.microsoft.com/en-us/cpp/c-runtime-library/complex-math-support
2: https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?c-standard-library-features-1
Change-Id: Iad8f26984b115c728e3b73f3a8334ade7a11cfa1
---
M src/cmd/cgo/out.go
1 file changed, 55 insertions(+), 0 deletions(-)
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser.
1 comment:
Patchset:
13 of 29 TryBots failed. […]
Implementation updated so it only affects the export headers.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Patch set 7:Run-TryBot +1
Attention is currently required from: Tobias Klauser, Quim Muntal.
1 comment:
File src/cmd/cgo/out.go:
Patch Set #7, Line 1426: case "complex float":
It's not clear to me where the "complex float" and "complex double" names come from.
Can we add a test case to misc/cgo/test/test.go that fails today with MSVC and succeeds after this patch is applied? It probably does't need a runtime test.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Ian Lance Taylor.
1 comment:
File src/cmd/cgo/out.go:
Patch Set #7, Line 1426: case "complex float":
It's not clear to me where the "complex float" and "complex double" names come from. […]
They come from the [debug/dwarf](https://cs.opensource.google/go/go/+/refs/tags/go1.18:src/debug/dwarf/type.go;l=578-590) package. I'll try to find a clearer way to identify these types, else I'll add a comment pointing to the source.
I think windows builders have MSVC installer. If so, it won't be difficult to check that the exported headers compile successfully under MSVC.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Ian Lance Taylor.
Quim Muntal uploaded patch set #8 to this change.
cmd/go: cgo export header to be compatible with MSVC complex types
After CL 379474 has landed, the only remaining cgo export header
incompatibility with MSVC is the use of the _Complex macro,
which is not not supported in MSVC even when it is part of the ISO C99
standard (1).
Since MSVC 2015 (2), complex math are supported via _Fcomplex and
_Dcomplex, which are equivalent to float _Complex and double _Complex.
As MSVC and C complex types have the same memory layout, we should
be able to typedef GoComplex64 and GoComplex128 to the appropriate
type in MSVC.
It is important to note that this CL is not adding MSVC support to cgo.
C compilers should still be GCC-compatible.
This CL is about allowing to include, without further modifications,
a DLL export header generated by cgo, normally using Mingw-W64 compiler,
into a MSVC project. This was already possible if the export header
changes introduced in this CL were done outside cgo, either manually or
in a post-build script.
Fixes #36233
1: https://docs.microsoft.com/en-us/cpp/c-runtime-library/complex-math-support
2: https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?c-standard-library-features-1
Change-Id: Iad8f26984b115c728e3b73f3a8334ade7a11cfa1
---
M misc/cgo/testcshared/cshared_test.go
A misc/cgo/testcshared/testdata/issue36233/issue36233.go
M src/cmd/cgo/out.go
3 files changed, 137 insertions(+), 11 deletions(-)
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Ian Lance Taylor.
Quim Muntal uploaded patch set #9 to this change.
Attention is currently required from: Tobias Klauser, Ian Lance Taylor.
1 comment:
File src/cmd/cgo/out.go:
Patch Set #7, Line 1426: case "complex float":
They come from the [debug/dwarf](https://cs.opensource.google/go/go/+/refs/tags/go1. […]
I've found a way to generalize what I was trying to do. Instead of relying on the C.Repr value, which is muddy, I'm now using the predeclared goTypes map, which was already mapping complex64/complex128 Go types to the appropriate C types.
I've removed the MSVC comment, as we are no longer special-caseing complex types.
Note that as side effect, all `C.*` types that can directly be mapped to a Go type will be translated to the typedef defined in goTypes, not just complex types. For example, `func Add(_ C.ulonglong) {}` was previously translated to `void Add(unsigned long long _);`, and now it will be translated to `Add(GoUint64 _);`.
Both representations are compatible, so there should be any compatibility issue. Still take it into account when reviewing.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Patch set 9:Run-TryBot +1
Attention is currently required from: Tobias Klauser, Quim Muntal.
Quim Muntal uploaded patch set #10 to this change.
3 files changed, 143 insertions(+), 11 deletions(-)
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Patchset:
2 of 29 TryBots failed. […]
Going back to special case complex types.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Patch set 10:Run-TryBot +1
Attention is currently required from: Tobias Klauser, Quim Muntal.
2 comments:
File misc/cgo/testcshared/cshared_test.go:
Patch Set #10, Line 878: t.Errorf("signature should contain: %q, got: %q", fn.signature, b)
Our usual convention is got %q want %q.
File src/cmd/cgo/out.go:
Patch Set #7, Line 1426: case "complex float":
I've found a way to generalize what I was trying to do. Instead of relying on the C. […]
Ack
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Quim Muntal uploaded patch set #11 to this change.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
File misc/cgo/testcshared/cshared_test.go:
Patch Set #10, Line 878: t.Errorf("signature should contain: %q, got: %q", fn.signature, b)
Our usual convention is got %q want %q.
Done
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Patch set 11:Run-TryBot +1Auto-Submit +1Code-Review +2
1 comment:
Patchset:
Thanks.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Patch set 11:Trust +1
1 comment:
Commit Message:
Patch Set #11, Line 11: not not
Typo.
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Quim Muntal uploaded patch set #12 to this change.
cmd/go: cgo export header to be compatible with MSVC complex types
After CL 379474 has landed, the only remaining cgo export header
incompatibility with MSVC is the use of the _Complex macro,
which is not supported in MSVC even when it is part of the ISO C99
standard (1).
Since MSVC 2015 (2), complex math are supported via _Fcomplex and
_Dcomplex, which are equivalent to float _Complex and double _Complex.
As MSVC and C complex types have the same memory layout, we should
be able to typedef GoComplex64 and GoComplex128 to the appropriate
type in MSVC.
It is important to note that this CL is not adding MSVC support to cgo.
C compilers should still be GCC-compatible.
This CL is about allowing to include, without further modifications,
a DLL export header generated by cgo, normally using Mingw-W64 compiler,
into a MSVC project. This was already possible if the export header
changes introduced in this CL were done outside cgo, either manually or
in a post-build script.
Fixes #36233
1: https://docs.microsoft.com/en-us/cpp/c-runtime-library/complex-math-support
2: https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?c-standard-library-features-1
Change-Id: Iad8f26984b115c728e3b73f3a8334ade7a11cfa1
---
M misc/cgo/testcshared/cshared_test.go
A misc/cgo/testcshared/testdata/issue36233/issue36233.go
M src/cmd/cgo/out.go
3 files changed, 143 insertions(+), 11 deletions(-)
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Cherry Mui.
1 comment:
Commit Message:
Patch Set #11, Line 11: not not
Typo.
Done
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Tobias Klauser, Quim Muntal.
Patch set 12:Run-TryBot +1Auto-Submit +1Trust +1
Gopher Robot submitted this change.
11 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
cmd/go: cgo export header to be compatible with MSVC complex types
After CL 379474 has landed, the only remaining cgo export header
incompatibility with MSVC is the use of the _Complex macro,
which is not supported in MSVC even when it is part of the ISO C99
standard (1).
Since MSVC 2015 (2), complex math are supported via _Fcomplex and
_Dcomplex, which are equivalent to float _Complex and double _Complex.
As MSVC and C complex types have the same memory layout, we should
be able to typedef GoComplex64 and GoComplex128 to the appropriate
type in MSVC.
It is important to note that this CL is not adding MSVC support to cgo.
C compilers should still be GCC-compatible.
This CL is about allowing to include, without further modifications,
a DLL export header generated by cgo, normally using Mingw-W64 compiler,
into a MSVC project. This was already possible if the export header
changes introduced in this CL were done outside cgo, either manually or
in a post-build script.
Fixes #36233
1: https://docs.microsoft.com/en-us/cpp/c-runtime-library/complex-math-support
2: https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?c-standard-library-features-1
Change-Id: Iad8f26984b115c728e3b73f3a8334ade7a11cfa1
Reviewed-on: https://go-review.googlesource.com/c/go/+/397134
Reviewed-by: Ian Lance Taylor <ia...@golang.org>
Trust: Cherry Mui <cher...@google.com>
Run-TryBot: Cherry Mui <cher...@google.com>
Auto-Submit: Cherry Mui <cher...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M misc/cgo/testcshared/cshared_test.go
A misc/cgo/testcshared/testdata/issue36233/issue36233.go
M src/cmd/cgo/out.go
3 files changed, 149 insertions(+), 11 deletions(-)
diff --git a/misc/cgo/testcshared/cshared_test.go b/misc/cgo/testcshared/cshared_test.go
index c9e9e5f..e489877 100644
--- a/misc/cgo/testcshared/cshared_test.go
+++ b/misc/cgo/testcshared/cshared_test.go
@@ -5,6 +5,7 @@
package cshared_test
import (
+ "bufio"
"bytes"
"debug/elf"
"debug/pe"
@@ -838,3 +839,51 @@
run(t, goenv, "go", "build", "-o", bin, "./go2c2go/m2")
runExe(t, runenv, bin)
}
+
+func TestIssue36233(t *testing.T) {
+ t.Parallel()
+
+ // Test that the export header uses GoComplex64 and GoComplex128
+ // for complex types.
+
+ tmpdir, err := os.MkdirTemp("", "cshared-TestIssue36233")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer os.RemoveAll(tmpdir)
+
+ const exportHeader = "issue36233.h"
+
+ run(t, nil, "go", "tool", "cgo", "-exportheader", exportHeader, "-objdir", tmpdir, "./issue36233/issue36233.go")
+ data, err := os.ReadFile(exportHeader)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ funcs := []struct{ name, signature string }{
+ {"exportComplex64", "GoComplex64 exportComplex64(GoComplex64 v)"},
+ {"exportComplex128", "GoComplex128 exportComplex128(GoComplex128 v)"},
+ {"exportComplexfloat", "GoComplex64 exportComplexfloat(GoComplex64 v)"},
+ {"exportComplexdouble", "GoComplex128 exportComplexdouble(GoComplex128 v)"},
+ }
+
+ scanner := bufio.NewScanner(bytes.NewReader(data))
+ var found int
+ for scanner.Scan() {
+ b := scanner.Bytes()
+ for _, fn := range funcs {
+ if bytes.Contains(b, []byte(fn.name)) {
+ found++
+ if !bytes.Contains(b, []byte(fn.signature)) {
+ t.Errorf("function signature mismatch; got %q, want %q", b, fn.signature)
+ }
+ }
+ }
+ }
+ if err = scanner.Err(); err != nil {
+ t.Errorf("scanner encountered error: %v", err)
+ }
+ if found != len(funcs) {
+ t.Error("missing functions")
+ }
+}
diff --git a/misc/cgo/testcshared/testdata/issue36233/issue36233.go b/misc/cgo/testcshared/testdata/issue36233/issue36233.go
new file mode 100644
index 0000000..d0d1e5d
--- /dev/null
+++ b/misc/cgo/testcshared/testdata/issue36233/issue36233.go
@@ -0,0 +1,29 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+package main
+
+// #include <complex.h>
+import "C"
+
+//export exportComplex64
+func exportComplex64(v complex64) complex64 {
+ return v
+}
+
+//export exportComplex128
+func exportComplex128(v complex128) complex128 {
+ return v
+}
+
+//export exportComplexfloat
+func exportComplexfloat(v C.complexfloat) C.complexfloat {
+ return v
+}
+
+//export exportComplexdouble
+func exportComplexdouble(v C.complexdouble) C.complexdouble {
+ return v
+}
+
+func main() {}
diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go
index 8ead173..adbb761 100644
--- a/src/cmd/cgo/out.go
+++ b/src/cmd/cgo/out.go
@@ -1399,6 +1399,19 @@
case *ast.ChanType:
return &Type{Size: p.PtrSize, Align: p.PtrSize, C: c("GoChan")}
case *ast.Ident:
+ goTypesFixup := func(r *Type) *Type {
+ if r.Size == 0 { // int or uint
+ rr := new(Type)
+ *rr = *r
+ rr.Size = p.IntSize
+ rr.Align = p.IntSize
+ r = rr
+ }
+ if r.Align > p.PtrSize {
+ r.Align = p.PtrSize
+ }
+ return r
+ }
// Look up the type in the top level declarations.
// TODO: Handle types defined within a function.
for _, d := range p.Decl {
@@ -1417,6 +1430,17 @@
}
}
if def := typedef[t.Name]; def != nil {
+ if defgo, ok := def.Go.(*ast.Ident); ok {
+ switch defgo.Name {
+ case "complex64", "complex128":
+ // MSVC does not support the _Complex keyword
+ // nor the complex macro.
+ // Use GoComplex64 and GoComplex128 instead,
+ // which are typedef-ed to a compatible type.
+ // See go.dev/issues/36233.
+ return goTypesFixup(goTypes[defgo.Name])
+ }
+ }
return def
}
if t.Name == "uintptr" {
@@ -1430,17 +1454,7 @@
return &Type{Size: 2 * p.PtrSize, Align: p.PtrSize, C: c("GoInterface")}
}
if r, ok := goTypes[t.Name]; ok {
- if r.Size == 0 { // int or uint
- rr := new(Type)
- *rr = *r
- rr.Size = p.IntSize
- rr.Align = p.IntSize
- r = rr
- }
- if r.Align > p.PtrSize {
- r.Align = p.PtrSize
- }
- return r
+ return goTypesFixup(r)
}
error_(e.Pos(), "unrecognized Go type %s", t.Name)
return &Type{Size: 4, Align: 4, C: c("int")}
@@ -1895,8 +1909,14 @@
typedef size_t GoUintptr;
typedef float GoFloat32;
typedef double GoFloat64;
+#ifdef _MSC_VER
+#include <complex.h>
+typedef _Fcomplex GoComplex64;
+typedef _Dcomplex GoComplex128;
+#else
typedef float _Complex GoComplex64;
typedef double _Complex GoComplex128;
+#endif
/*
static assertion to make sure the file is being used on architecture
To view, visit change 397134. To unsubscribe, or for help writing mail filters, visit settings.