[go] cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Cuong Manh Le (Gerrit)
Cuong Manh Le has uploaded this change for review.
cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Updates #46731
Change-Id: Ia83f27c177cc2f57e240cb5c6708d4552423f5be
---
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/cgo/out.go
M src/runtime/cgo/cgo.go
4 files changed, 35 insertions(+), 48 deletions(-)
diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
index d89bff2..e7bde79 100644
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -2154,9 +2154,6 @@
// Type names X for which there exists an XGetTypeID function with type func() CFTypeID.
getTypeIDs map[string]bool
- // badStructs contains C structs that should be marked NotInHeap.
- notInHeapStructs map[string]bool
-
// Predeclared types.
bool ast.Expr
byte ast.Expr // denotes padding
@@ -2168,7 +2165,6 @@
string ast.Expr
goVoid ast.Expr // _Ctype_void, denotes C's void
goVoidPtr ast.Expr // unsafe.Pointer or *byte
- goVoidPtrNoHeap ast.Expr // *_Ctype_void_notinheap, like goVoidPtr but marked NotInHeap
ptrSize int64
intSize int64
@@ -2192,7 +2188,6 @@
c.m = make(map[string]*Type)
c.ptrs = make(map[string][]*Type)
c.getTypeIDs = make(map[string]bool)
- c.notInHeapStructs = make(map[string]bool)
c.bool = c.Ident("bool")
c.byte = c.Ident("byte")
c.int8 = c.Ident("int8")
@@ -2211,7 +2206,6 @@
c.void = c.Ident("void")
c.string = c.Ident("string")
c.goVoid = c.Ident("_Ctype_void")
- c.goVoidPtrNoHeap = c.Ident("*_Ctype_void_notinheap")
// Normally cgo translates void* to unsafe.Pointer,
// but for historical reasons -godefs uses *byte instead.
@@ -2561,19 +2555,13 @@
// other than try to determine a Go representation.
tt := *t
tt.C = &TypeRepr{"%s %s", []interface{}{dt.Kind, tag}}
- tt.Go = c.Ident("struct{}")
- if dt.Kind == "struct" {
- // We don't know what the representation of this struct is, so don't let
- // anyone allocate one on the Go side. As a side effect of this annotation,
- // pointers to this type will not be considered pointers in Go. They won't
- // get writebarrier-ed or adjusted during a stack copy. This should handle
- // all the cases badPointerTypedef used to handle, but hopefully will
- // continue to work going forward without any more need for cgo changes.
- tt.NotInHeap = true
- // TODO: we should probably do the same for unions. Unions can't live
- // on the Go heap, right? It currently doesn't work for unions because
- // they are defined as a type alias for struct{}, not a defined type.
- }
+ // We don't know what the representation of this struct is, so don't let
+ // anyone allocate one on the Go side. As a side effect of this annotation,
+ // pointers to this type will not be considered pointers in Go. They won't
+ // get writebarrier-ed or adjusted during a stack copy. This should handle
+ // all the cases badPointerTypedef used to handle, but hopefully will
+ // continue to work going forward without any more need for cgo changes.
+ tt.Go = c.Ident("_Cgopkg.Incomplete")
typedef[name.Name] = &tt
break
}
@@ -2599,7 +2587,6 @@
tt.C = &TypeRepr{"struct %s", []interface{}{tag}}
}
tt.Go = g
- tt.NotInHeap = c.notInHeapStructs[tag]
typedef[name.Name] = &tt
}
@@ -2644,32 +2631,17 @@
}
}
if c.badVoidPointerTypedef(dt) {
- // Treat this typedef as a pointer to a NotInHeap void.
+ // Treat this typedef as a pointer to a _Cgopkg.Incomplete.
s := *sub
- s.Go = c.goVoidPtrNoHeap
+ s.Go = c.Ident("*_Cgopkg.Incomplete")
sub = &s
// Make sure we update any previously computed type.
if oldType := typedef[name.Name]; oldType != nil {
oldType.Go = sub.Go
}
}
- // Check for non-pointer "struct <tag>{...}; typedef struct <tag> *<name>"
- // typedefs that should be marked NotInHeap.
- if ptr, ok := dt.Type.(*dwarf.PtrType); ok {
- if strct, ok := ptr.Type.(*dwarf.StructType); ok {
- if c.badStructPointerTypedef(dt.Name, strct) {
- c.notInHeapStructs[strct.StructName] = true
- // Make sure we update any previously computed type.
- name := "_Ctype_struct_" + strct.StructName
- if oldType := typedef[name]; oldType != nil {
- oldType.NotInHeap = true
- }
- }
- }
- }
t.Go = name
t.BadPointer = sub.BadPointer
- t.NotInHeap = sub.NotInHeap
if unionWithPointer[sub.Go] {
unionWithPointer[t.Go] = true
}
@@ -2680,7 +2652,6 @@
tt := *t
tt.Go = sub.Go
tt.BadPointer = sub.BadPointer
- tt.NotInHeap = sub.NotInHeap
typedef[name.Name] = &tt
}
@@ -3204,7 +3175,7 @@
// non-pointers in this type.
// TODO: Currently our best solution is to find these manually and list them as
// they come up. A better solution is desired.
-// Note: DEPRECATED. There is now a better solution. Search for NotInHeap in this file.
+// Note: DEPRECATED. There is now a better solution. Search for _Cgopkg.Incomplete in this file.
func (c *typeConv) badPointerTypedef(dt *dwarf.TypedefType) bool {
if c.badCFType(dt) {
return true
@@ -3218,7 +3189,7 @@
return false
}
-// badVoidPointerTypedef is like badPointerTypeDef, but for "void *" typedefs that should be NotInHeap.
+// badVoidPointerTypedef is like badPointerTypeDef, but for "void *" typedefs that should be _Cgopkg.Incomplete.
func (c *typeConv) badVoidPointerTypedef(dt *dwarf.TypedefType) bool {
// Match the Windows HANDLE type (#42018).
if goos != "windows" || dt.Name != "HANDLE" {
diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go
index e343459..4f94d77 100644
--- a/src/cmd/cgo/main.go
+++ b/src/cmd/cgo/main.go
@@ -153,7 +153,6 @@
EnumValues map[string]int64
Typedef string
BadPointer bool // this pointer type should be represented as a uintptr (deprecated)
- NotInHeap bool // this type should have a go:notinheap annotation
}
// A FuncType collects information about a function type in both the C and Go worlds.
diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go
index 119eca2..0708589 100644
--- a/src/cmd/cgo/out.go
+++ b/src/cmd/cgo/out.go
@@ -81,11 +81,14 @@
fmt.Fprintf(fgo2, "// Code generated by cmd/cgo; DO NOT EDIT.\n\n")
fmt.Fprintf(fgo2, "package %s\n\n", p.PackageName)
fmt.Fprintf(fgo2, "import \"unsafe\"\n\n")
- if !*gccgo && *importRuntimeCgo {
- fmt.Fprintf(fgo2, "import _ \"runtime/cgo\"\n\n")
- }
if *importSyscall {
fmt.Fprintf(fgo2, "import \"syscall\"\n\n")
+ }
+ if !*gccgo && *importRuntimeCgo {
+ fmt.Fprintf(fgo2, "import _Cgopkg \"runtime/cgo\"\n\n")
+ fmt.Fprintf(fgo2, "type _ _Cgopkg.Incomplete\n")
+ }
+ if *importSyscall {
fmt.Fprintf(fgo2, "var _ syscall.Errno\n")
}
fmt.Fprintf(fgo2, "func _Cgo_ptr(ptr unsafe.Pointer) unsafe.Pointer { return ptr }\n\n")
@@ -109,9 +112,6 @@
sort.Strings(typedefNames)
for _, name := range typedefNames {
def := typedef[name]
- if def.NotInHeap {
- fmt.Fprintf(fgo2, "//go:notinheap\n")
- }
fmt.Fprintf(fgo2, "type %s ", name)
// We don't have source info for these types, so write them out without source info.
// Otherwise types would look like:
@@ -136,7 +136,6 @@
fmt.Fprintf(fgo2, "%s", buf.Bytes())
fmt.Fprintf(fgo2, "\n\n")
}
- fmt.Fprintf(fgo2, "//go:notinheap\ntype _Ctype_void_notinheap struct{}\n\n")
if *gccgo {
fmt.Fprintf(fgo2, "type _Ctype_void byte\n")
} else {
diff --git a/src/runtime/cgo/cgo.go b/src/runtime/cgo/cgo.go
index 298aa63..6d721bc 100644
--- a/src/runtime/cgo/cgo.go
+++ b/src/runtime/cgo/cgo.go
@@ -29,3 +29,10 @@
*/
import "C"
+
+import "runtime/internal/sys"
+
+// Incomplete is used specifically for the semantics of incomplete C types.
+type Incomplete struct {
+ _ sys.NotInHeap
+}
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Patch set 1:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Cuong Manh Le.
Cuong Manh Le uploaded patch set #2 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Cuong Manh Le, TryBot-Result-1 by Gopher Robot
cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Updates #46731
Change-Id: Ia83f27c177cc2f57e240cb5c6708d4552423f5be
---
A api/next/46731.txt
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/cgo/out.go
M src/runtime/cgo/cgo.go
5 files changed, 37 insertions(+), 48 deletions(-)
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Patch set 2:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Cuong Manh Le.
Cuong Manh Le uploaded patch set #3 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Cuong Manh Le, TryBot-Result-1 by Gopher Robot
cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Updates #46731
Change-Id: Ia83f27c177cc2f57e240cb5c6708d4552423f5be
---
A api/next/46731.txt
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/cgo/out.go
M src/runtime/cgo/cgo.go
5 files changed, 46 insertions(+), 36 deletions(-)
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Patch set 3:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Cuong Manh Le.
Cuong Manh Le uploaded patch set #4 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Cuong Manh Le
cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Updates #46731
Change-Id: Ia83f27c177cc2f57e240cb5c6708d4552423f5be
---
A api/next/46731.txt
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/cgo/out.go
M src/runtime/cgo/cgo.go
5 files changed, 59 insertions(+), 36 deletions(-)
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Patch set 4:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Keith Randall, Matthew Dempsky.
Patch set 5:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Keith Randall, Matthew Dempsky.
Patch set 6:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Keith Randall, Matthew Dempsky.
Patch set 7:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Cuong Manh Le, Keith Randall, Matthew Dempsky.
Cuong Manh Le uploaded patch set #8 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Cuong Manh Le, TryBot-Result+1 by Gopher Robot
cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Updates #46731
Change-Id: Ia83f27c177cc2f57e240cb5c6708d4552423f5be
---
A api/next/46731.txt
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/cgo/out.go
M src/runtime/cgo/cgo.go
5 files changed, 58 insertions(+), 36 deletions(-)
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Keith Randall, Matthew Dempsky.
Patch set 8:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Keith Randall, Matthew Dempsky.
Patch set 9:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Keith Randall, Matthew Dempsky.
Patch set 10:Run-TryBot +1
Keith Randall (Gerrit)
Attention is currently required from: Cuong Manh Le, Ian Lance Taylor, Matthew Dempsky.
Patch set 10:Code-Review +1
3 comments:
Patchset:
Ian, could you take a look at this one? I'm not sure what exporting API from this package could do.
(The other option, I think, is to generate an internal incomplete type in each cgo output file.)
File src/cmd/cgo/gcc.go:
I'd rather call these incompleteStructs. "bad" is too vague.
File src/cmd/cgo/out.go:
Patch Set #10, Line 89:
fmt.Fprintf(fgo2, "type _ _Cgopkg.Incomplete\n")Comment that this line is just to prevent import-not-used errors.
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Cuong Manh Le, Keith Randall, Matthew Dempsky.
4 comments:
Patchset:
Ian, could you take a look at this one? I'm not sure what exporting API from this package could do. […]
As far as I can tell this should be OK. We already import the package anyhow.
File api/next/46731.txt:
Patch Set #10, Line 1:
pkg runtime/cgo (darwin-amd64-cgo), type Incomplete struct #46731Why are we listing these many times for different targets? Can't we just list it once?
File src/cmd/cgo/out.go:
Patch Set #10, Line 87:
if !*gccgo && *importRuntimeCgo {We should drop the "if !*gccgo" now, since we're going to need to refer to the package in some cases.
Patch Set #10, Line 88:
fmt.Fprintf(fgo2, "import _Cgopkg \"runtime/cgo\"\n\n")Why we are calling this "_Cgopkg"? Package names are normally all lowercase. We have a bunch of _cgo names already. How about "_cgopackage"?
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Cuong Manh Le, Keith Randall, Matthew Dempsky.
Cuong Manh Le uploaded patch set #11 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Cuong Manh Le, TryBot-Result+1 by Gopher Robot
cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Updates #46731
Change-Id: Ia83f27c177cc2f57e240cb5c6708d4552423f5be
---
A api/next/46731.txt
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/cgo/out.go
M src/runtime/cgo/cgo.go
5 files changed, 58 insertions(+), 36 deletions(-)
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Cuong Manh Le, Keith Randall, Matthew Dempsky.
Cuong Manh Le uploaded patch set #12 to this change.
cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Updates #46731
Change-Id: Ia83f27c177cc2f57e240cb5c6708d4552423f5be
---
A api/next/46731.txt
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/cgo/out.go
M src/runtime/cgo/cgo.go
5 files changed, 58 insertions(+), 36 deletions(-)
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Ian Lance Taylor, Keith Randall, Matthew Dempsky.
Patch set 12:Run-TryBot +1
5 comments:
File api/next/46731.txt:
Patch Set #10, Line 1:
pkg runtime/cgo (darwin-amd64-cgo), type Incomplete struct #46731Why are we listing these many times for different targets? Can't we just list it once?
Yes, I guess because the API only introduced in some targets, not all, so we need to list them all. Otherwise, cmd/api would complain.
File src/cmd/cgo/gcc.go:
I'd rather call these incompleteStructs. "bad" is too vague.
Done
File src/cmd/cgo/out.go:
Patch Set #10, Line 87:
if !*gccgo && *importRuntimeCgo {
We should drop the "if !*gccgo" now, since we're going to need to refer to the package in some cases […]
Done
Patch Set #10, Line 88:
fmt.Fprintf(fgo2, "import _Cgopkg \"runtime/cgo\"\n\n")
Why we are calling this "_Cgopkg"? Package names are normally all lowercase. […]
No special reason, I just use the same name as Matthew's suggestion in the issue. Updated.
Patch Set #10, Line 89:
fmt.Fprintf(fgo2, "type _ _Cgopkg.Incomplete\n")Comment that this line is just to prevent import-not-used errors.
Done
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Ian Lance Taylor, Keith Randall, Matthew Dempsky.
Patch set 13:Run-TryBot +1
Cuong Manh Le (Gerrit)
Attention is currently required from: Ian Lance Taylor, Keith Randall, Matthew Dempsky.
1 comment:
File api/next/46731.txt:
Patch Set #10, Line 1:
pkg runtime/cgo (darwin-amd64-cgo), type Incomplete struct #46731
Yes, I guess because the API only introduced in some targets, not all, so we need to list them all. […]
So targets that are CGO enabled listed here https://github.com/golang/go/blob/9f0f87c806b7a11b2cb3ebcd02eac57ee389c43a/src/cmd/api/goapi.go#L59, and they're considered separated target. That's why we need to list them all.
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Dempsky (Gerrit)
Attention is currently required from: Cuong Manh Le, Ian Lance Taylor, Keith Randall.
Patch set 13:Code-Review +2
2 comments:
Patchset:
Thanks Cuong.
File api/next/46731.txt:
Patch Set #10, Line 1:
pkg runtime/cgo (darwin-amd64-cgo), type Incomplete struct #46731
So targets that are CGO enabled listed here https://github. […]
I agree with Ian that it seems weird we need to list these multiple times, but that does seem to be what cmd/api is requiring. I think for this CL it's fine to just appease cmd/api's current logic. I've filed https://github.com/golang/go/issues/54657 to investigate why cmd/api doesn't allow a single line here.
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Ian Lance Taylor, Keith Randall.
1 comment:
Patchset:
So sounds like @golang.org is not considered Google (even Keith is a Googler), should I tag only @google.com mail from now on?
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Dempsky (Gerrit)
Attention is currently required from: Cuong Manh Le, Ian Lance Taylor, Keith Randall.
1 comment:
Patchset:
So sounds like @golang. […]
The whole situation is a big mess, sorry.
I think for now though folks with @golang.org accounts prefer to continue having reviews sent to those addresses, and they'll additionally +1 from their @google.com accounts as appropriate.
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Ian Lance Taylor, Keith Randall.
1 comment:
Patchset:
Kindly ping Keith, since we need +1 from your google account.
RELNOTE=yes
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
Attention is currently required from: Ian Lance Taylor, Keith Randall.
Patch set 14:Run-TryBot +1
David Chase (Gerrit)
Attention is currently required from: Cuong Manh Le, Ian Lance Taylor, Keith Randall.
Patch set 14:Code-Review +1
Cuong Manh Le (Gerrit)
Cuong Manh Le submitted this change.
13 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
cmd/cgo: add and use runtime/cgo.Incomplete instead of //go:notinheap
Updates #46731
Change-Id: Ia83f27c177cc2f57e240cb5c6708d4552423f5be
Reviewed-on: https://go-review.googlesource.com/c/go/+/421879
Run-TryBot: Cuong Manh Le <cuong.m...@gmail.com>
Reviewed-by: Matthew Dempsky <mdem...@google.com>
Reviewed-by: Keith Randall <k...@golang.org>
Reviewed-by: David Chase <drc...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
A api/next/46731.txt
M src/cmd/cgo/gcc.go
M src/cmd/cgo/main.go
M src/cmd/cgo/out.go
M src/runtime/cgo/cgo.go
5 files changed, 64 insertions(+), 36 deletions(-)
diff --git a/api/next/46731.txt b/api/next/46731.txt
new file mode 100644
index 0000000..1d491ef
--- /dev/null
+++ b/api/next/46731.txt
@@ -0,0 +1,14 @@
+pkg runtime/cgo (darwin-amd64-cgo), type Incomplete struct #46731
+pkg runtime/cgo (freebsd-386-cgo), type Incomplete struct #46731
+pkg runtime/cgo (freebsd-amd64-cgo), type Incomplete struct #46731
+pkg runtime/cgo (freebsd-arm-cgo), type Incomplete struct #46731
+pkg runtime/cgo (linux-386-cgo), type Incomplete struct #46731
+pkg runtime/cgo (linux-amd64-cgo), type Incomplete struct #46731
+pkg runtime/cgo (linux-arm-cgo), type Incomplete struct #46731
+pkg runtime/cgo (netbsd-386-cgo), type Incomplete struct #46731
+pkg runtime/cgo (netbsd-amd64-cgo), type Incomplete struct #46731
+pkg runtime/cgo (netbsd-arm-cgo), type Incomplete struct #46731
+pkg runtime/cgo (netbsd-arm64-cgo), type Incomplete struct #46731
+pkg runtime/cgo (openbsd-386-cgo), type Incomplete struct #46731
+pkg runtime/cgo (openbsd-amd64-cgo), type Incomplete struct #46731
+pkg runtime/cgo, type Incomplete struct #46731
diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
index d89bff2..43f94bb 100644
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -2154,8 +2154,8 @@
// Type names X for which there exists an XGetTypeID function with type func() CFTypeID.
getTypeIDs map[string]bool
- // badStructs contains C structs that should be marked NotInHeap.
- notInHeapStructs map[string]bool
+ // incompleteStructs contains C structs that should be marked Incomplete.
+ incompleteStructs map[string]bool
// Predeclared types.
bool ast.Expr
@@ -2168,7 +2168,6 @@
string ast.Expr
goVoid ast.Expr // _Ctype_void, denotes C's void
goVoidPtr ast.Expr // unsafe.Pointer or *byte
- goVoidPtrNoHeap ast.Expr // *_Ctype_void_notinheap, like goVoidPtr but marked NotInHeap
ptrSize int64
intSize int64
@@ -2192,7 +2191,7 @@
c.m = make(map[string]*Type)
c.ptrs = make(map[string][]*Type)
c.getTypeIDs = make(map[string]bool)
- c.notInHeapStructs = make(map[string]bool)
+ c.incompleteStructs = make(map[string]bool)
c.bool = c.Ident("bool")
c.byte = c.Ident("byte")
c.int8 = c.Ident("int8")
@@ -2211,7 +2210,6 @@
c.void = c.Ident("void")
c.string = c.Ident("string")
c.goVoid = c.Ident("_Ctype_void")
- c.goVoidPtrNoHeap = c.Ident("*_Ctype_void_notinheap")
// Normally cgo translates void* to unsafe.Pointer,
// but for historical reasons -godefs uses *byte instead.
@@ -2561,19 +2559,13 @@
// other than try to determine a Go representation.
tt := *t
tt.C = &TypeRepr{"%s %s", []interface{}{dt.Kind, tag}}
- tt.Go = c.Ident("struct{}")
- if dt.Kind == "struct" {
- // We don't know what the representation of this struct is, so don't let
- // anyone allocate one on the Go side. As a side effect of this annotation,
- // pointers to this type will not be considered pointers in Go. They won't
- // get writebarrier-ed or adjusted during a stack copy. This should handle
- // all the cases badPointerTypedef used to handle, but hopefully will
- // continue to work going forward without any more need for cgo changes.
- tt.NotInHeap = true
- // TODO: we should probably do the same for unions. Unions can't live
- // on the Go heap, right? It currently doesn't work for unions because
- // they are defined as a type alias for struct{}, not a defined type.
- }
+ // We don't know what the representation of this struct is, so don't let
+ // anyone allocate one on the Go side. As a side effect of this annotation,
+ // pointers to this type will not be considered pointers in Go. They won't
+ // get writebarrier-ed or adjusted during a stack copy. This should handle
+ // all the cases badPointerTypedef used to handle, but hopefully will
+ // continue to work going forward without any more need for cgo changes.
+ tt.Go = c.Ident("_cgopackage.Incomplete")
typedef[name.Name] = &tt
break
}
@@ -2599,7 +2591,9 @@
tt.C = &TypeRepr{"struct %s", []interface{}{tag}}
}
tt.Go = g
- tt.NotInHeap = c.notInHeapStructs[tag]
+ if c.incompleteStructs[tag] {
+ tt.Go = c.Ident("_cgopackage.Incomplete")
+ }
typedef[name.Name] = &tt
}
@@ -2644,9 +2638,9 @@
}
}
if c.badVoidPointerTypedef(dt) {
- // Treat this typedef as a pointer to a NotInHeap void.
+ // Treat this typedef as a pointer to a _cgopackage.Incomplete.
s := *sub
- s.Go = c.goVoidPtrNoHeap
+ s.Go = c.Ident("*_cgopackage.Incomplete")
sub = &s
// Make sure we update any previously computed type.
if oldType := typedef[name.Name]; oldType != nil {
@@ -2654,22 +2648,21 @@
}
}
// Check for non-pointer "struct <tag>{...}; typedef struct <tag> *<name>"
- // typedefs that should be marked NotInHeap.
+ // typedefs that should be marked Incomplete.
if ptr, ok := dt.Type.(*dwarf.PtrType); ok {
if strct, ok := ptr.Type.(*dwarf.StructType); ok {
if c.badStructPointerTypedef(dt.Name, strct) {
- c.notInHeapStructs[strct.StructName] = true
+ c.incompleteStructs[strct.StructName] = true
// Make sure we update any previously computed type.
name := "_Ctype_struct_" + strct.StructName
if oldType := typedef[name]; oldType != nil {
- oldType.NotInHeap = true
+ oldType.Go = c.Ident("_cgopackage.Incomplete")
}
}
}
}
t.Go = name
t.BadPointer = sub.BadPointer
- t.NotInHeap = sub.NotInHeap
if unionWithPointer[sub.Go] {
unionWithPointer[t.Go] = true
}
@@ -2680,7 +2673,6 @@
tt := *t
tt.Go = sub.Go
tt.BadPointer = sub.BadPointer
- tt.NotInHeap = sub.NotInHeap
typedef[name.Name] = &tt
}
@@ -3204,7 +3196,7 @@
// non-pointers in this type.
// TODO: Currently our best solution is to find these manually and list them as
// they come up. A better solution is desired.
-// Note: DEPRECATED. There is now a better solution. Search for NotInHeap in this file.
+// Note: DEPRECATED. There is now a better solution. Search for _cgopackage.Incomplete in this file.
func (c *typeConv) badPointerTypedef(dt *dwarf.TypedefType) bool {
if c.badCFType(dt) {
return true
@@ -3218,7 +3210,7 @@
return false
}
-// badVoidPointerTypedef is like badPointerTypeDef, but for "void *" typedefs that should be NotInHeap.
+// badVoidPointerTypedef is like badPointerTypeDef, but for "void *" typedefs that should be _cgopackage.Incomplete.
func (c *typeConv) badVoidPointerTypedef(dt *dwarf.TypedefType) bool {
// Match the Windows HANDLE type (#42018).
if goos != "windows" || dt.Name != "HANDLE" {
diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go
index e343459..4f94d77 100644
--- a/src/cmd/cgo/main.go
+++ b/src/cmd/cgo/main.go
@@ -153,7 +153,6 @@
EnumValues map[string]int64
Typedef string
BadPointer bool // this pointer type should be represented as a uintptr (deprecated)
- NotInHeap bool // this type should have a go:notinheap annotation
}
// A FuncType collects information about a function type in both the C and Go worlds.
diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go
index 119eca2..6a22459 100644
--- a/src/cmd/cgo/out.go
+++ b/src/cmd/cgo/out.go
@@ -81,11 +81,14 @@
fmt.Fprintf(fgo2, "// Code generated by cmd/cgo; DO NOT EDIT.\n\n")
fmt.Fprintf(fgo2, "package %s\n\n", p.PackageName)
fmt.Fprintf(fgo2, "import \"unsafe\"\n\n")
- if !*gccgo && *importRuntimeCgo {
- fmt.Fprintf(fgo2, "import _ \"runtime/cgo\"\n\n")
- }
if *importSyscall {
fmt.Fprintf(fgo2, "import \"syscall\"\n\n")
+ }
+ if *importRuntimeCgo {
+ fmt.Fprintf(fgo2, "import _cgopackage \"runtime/cgo\"\n\n")
+ fmt.Fprintf(fgo2, "type _ _cgopackage.Incomplete\n") // prevent import-not-used error
+ }
+ if *importSyscall {
fmt.Fprintf(fgo2, "var _ syscall.Errno\n")
}
fmt.Fprintf(fgo2, "func _Cgo_ptr(ptr unsafe.Pointer) unsafe.Pointer { return ptr }\n\n")
@@ -109,9 +112,6 @@
sort.Strings(typedefNames)
for _, name := range typedefNames {
def := typedef[name]
- if def.NotInHeap {
- fmt.Fprintf(fgo2, "//go:notinheap\n")
- }
fmt.Fprintf(fgo2, "type %s ", name)
// We don't have source info for these types, so write them out without source info.
// Otherwise types would look like:
@@ -136,7 +136,6 @@
fmt.Fprintf(fgo2, "%s", buf.Bytes())
fmt.Fprintf(fgo2, "\n\n")
}
- fmt.Fprintf(fgo2, "//go:notinheap\ntype _Ctype_void_notinheap struct{}\n\n")
if *gccgo {
fmt.Fprintf(fgo2, "type _Ctype_void byte\n")
} else {
diff --git a/src/runtime/cgo/cgo.go b/src/runtime/cgo/cgo.go
index 4b7046e..b8473e5 100644
--- a/src/runtime/cgo/cgo.go
+++ b/src/runtime/cgo/cgo.go
@@ -31,3 +31,10 @@
*/
import "C"
+
+import "runtime/internal/sys"
+
+// Incomplete is used specifically for the semantics of incomplete C types.
+type Incomplete struct {
+ _ sys.NotInHeap
+}
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Cuong Manh Le.
1 comment:
File src/cmd/cgo/out.go:
if *importRuntimeCgo {
fmt.Fprintf(fgo2, "import _cgopackage \"runtime/cgo\"\n\n")
fmt.Fprintf(fgo2, "type _ _cgopackage.Incomplete\n") // prevent import-not-used error
}
This change appears to have broken `cmd/go.TestScript/build_overlay` when `gccgo` is installed, because the `gccgo` version of the `runtime/cgo` package does not define the `Incomplete` type.
I'm not sure exactly what to do about that, though.
```
> go build -compiler=gccgo -overlay overlay.json -o main_cgo_replace_gccgo$GOEXE ./cgo_hello_replace
[stderr]
# m/cgo_hello_replace
$WORK/tmp/go-build718572163/b001/_cgo_gotypes.go:9:8: error: import file 'runtime/cgo' not found
9 | import _cgopackage "runtime/cgo"
| ^
$WORK/tmp/go-build718572163/b001/_cgo_gotypes.go:11:19: error: expected package
11 | type _ _cgopackage.Incomplete
| ^
[exit status 2]
FAIL: testdata/script/build_overlay.txt:87: unexpected command failure
```
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
File src/cmd/cgo/out.go:
if *importRuntimeCgo {
fmt.Fprintf(fgo2, "import _cgopackage \"runtime/cgo\"\n\n")
fmt.Fprintf(fgo2, "type _ _cgopackage.Incomplete\n") // prevent import-not-used error
}
This change appears to have broken `cmd/go. […]
Filed as #54761, and I've mailed CL 426496 to skip that part of the test for now.
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.
Cuong Manh Le (Gerrit)
1 comment:
File src/cmd/cgo/out.go:
if *importRuntimeCgo {
fmt.Fprintf(fgo2, "import _cgopackage \"runtime/cgo\"\n\n")
fmt.Fprintf(fgo2, "type _ _cgopackage.Incomplete\n") // prevent import-not-used error
}
Filed as #54761, and I've mailed CL 426496 to skip that part of the test for now.
Thanks, do restoring the check `if !*gccgo && *importRuntimeCgo` makes it works?
To view, visit change 421879. To unsubscribe, or for help writing mail filters, visit settings.