Skip to content

fix: replace deprecated reflect.SliceHeader with unsafe.Slice#366

Open
dlevy-msft-sql wants to merge 1 commit into
microsoft:mainfrom
dlevy-msft-sql:fix/replace-sliceheader
Open

fix: replace deprecated reflect.SliceHeader with unsafe.Slice#366
dlevy-msft-sql wants to merge 1 commit into
microsoft:mainfrom
dlevy-msft-sql:fix/replace-sliceheader

Conversation

@dlevy-msft-sql
Copy link
Copy Markdown

Problem

The UCS-2 string decoding optimization in ucs22str.go uses reflect.SliceHeader which was deprecated in Go 1.21. The deprecation notice recommends using unsafe.Slice instead.

Fix

Replace 7 lines of manual slice header manipulation:

var uint16slice []uint16
uint16Header := (*reflect.SliceHeader)(unsafe.Pointer(&uint16slice))
sourceHeader := (*reflect.SliceHeader)(unsafe.Pointer(&s))
uint16Header.Data = sourceHeader.Data
uint16Header.Len = len(s) / 2
uint16Header.Cap = uint16Header.Len

With a single unsafe.Slice call:

uint16slice := unsafe.Slice((*uint16)(unsafe.Pointer(&s[0])), len(s)/2)

This is equivalent behavior: both reinterpret the underlying []byte as []uint16 for the subsequent utf16.Decode call. The unsafe.Slice version is the recommended modern pattern and lets the compiler/GC properly track the pointer.

Tests

All 17 UCS-2 string conversion tests pass, covering ASCII (lengths 1-9+), Unicode, and emoji inputs.

Fixes #272

@dlevy-msft-sql dlevy-msft-sql added this to the v1.11.0 milestone Apr 17, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.62%. Comparing base (c10fa99) to head (1784275).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #366       +/-   ##
===========================================
+ Coverage   80.66%   96.62%   +15.95%     
===========================================
  Files          35       92       +57     
  Lines        6842    74273    +67431     
===========================================
+ Hits         5519    71764    +66245     
- Misses       1055     2176     +1121     
- Partials      268      333       +65     
Flag Coverage Δ
unittests 96.54% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
ucs22str.go 100.00% <100.00%> (ø)

... and 59 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dlevy-msft-sql dlevy-msft-sql force-pushed the fix/replace-sliceheader branch from 74022f2 to 5368a77 Compare April 24, 2026 21:04
reflect.SliceHeader was deprecated in Go 1.21. Replace the manual
slice header manipulation with unsafe.Slice which is the recommended
modern alternative (available since Go 1.17).

The new code is also simpler and clearer: a single unsafe.Slice call
replaces 7 lines of manual header field assignments.

Fixes microsoft#272
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the UCS-2 decoding fast/slow-path implementation to remove use of deprecated reflect.SliceHeader in favor of the modern unsafe.Slice API, aligning the driver with Go 1.21+ guidance while preserving the existing optimization approach.

Changes:

  • Removed the reflect import and manual reflect.SliceHeader manipulation in ucs22str.go.
  • Replaced the byte-slice-to-uint16-slice reinterpretation with a single unsafe.Slice call for the UTF-16 decode slow path.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace use of reflect.SliceHeader with unsafe.Slice

3 participants