Skip to content

Commit

Permalink
Improve unsafe reflect value handling.
Browse files Browse the repository at this point in the history
This commit modifies the unsafeReflectValue function to recognize
reference types even when the indirection flag is not set for the series
of golang commits after ecccf07e7f9d and before 82f48826c6c7 which
introduced the additional scalar field in the reflect.Value struct.  That
additional field has since been removed, but the intention of this code is
to work properly across all Go versions and other packages make use of the
logic.

Thanks to @shurcooL for providing example code which wasn't working
properly with the function when it was exported and therefore being called
in ways which spew itself does not.
  • Loading branch information
davecgh committed Nov 16, 2014
1 parent 1288542 commit 83f84dc
Showing 1 changed file with 13 additions and 1 deletion.
14 changes: 13 additions & 1 deletion spew/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,19 @@ func unsafeReflectValue(v reflect.Value) (rv reflect.Value) {
vt = reflect.PtrTo(v.Type())
indirects++
} else if offsetScalar != 0 {
upv = unsafe.Pointer(uintptr(unsafe.Pointer(&v)) + offsetScalar)
// The value is in the scalar field when it's not one of the
// reference types.
switch vt.Kind() {
case reflect.Uintptr:
case reflect.Chan:
case reflect.Func:
case reflect.Map:
case reflect.Ptr:
case reflect.UnsafePointer:
default:
upv = unsafe.Pointer(uintptr(unsafe.Pointer(&v)) +
offsetScalar)
}
}

pv := reflect.NewAt(vt, upv)
Expand Down

3 comments on commit 83f84dc

@dmitshur
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the intention of this code is to work properly across all Go versions and other packages make use of the logic.

Would it be accurate to say the intention is to work properly across all released Go versions? As in, 1.3.3, 1.4 (when the final version does come out), but not necessary in-between versions that are not released?

@davecgh
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the intent. That said, as far as I know, it does actually work across everything from 1.0 forward, including the in-between versions.

@dmitshur
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool.

As a heads up, this change fixes my reflection-based widget from:

image

To a working state:

Reflection and OpenGL LOD Bias.mov

Please sign in to comment.