Skip to content

feat(reflect): add Indirect function#1355

Merged
xushiwei merged 4 commits intomainfrom
feature/reflect-indirect
Oct 26, 2025
Merged

feat(reflect): add Indirect function#1355
xushiwei merged 4 commits intomainfrom
feature/reflect-indirect

Conversation

@xgopilot
Copy link
Contributor

@xgopilot xgopilot bot commented Oct 17, 2025

Summary

Implements reflect.Indirect function to support pointer dereferencing in llgo's reflect package.

Changes

  • Added Indirect function to /workspace/runtime/internal/lib/reflect/value.go
  • Created test case in _cmptest/reflect_indirect/test.go

Implementation

func Indirect(v Value) Value {
	if v.Kind() != Pointer {
		return v
	}
	return v.Elem()
}

Behavior

  • If v is a pointer (nil or non-nil): returns the dereferenced value via v.Elem()
  • If v is not a pointer: returns v unchanged

Testing

The test covers:

  1. Non-pointer values (should return unchanged)
  2. Pointer values (should dereference)
  3. Nil pointer values (should return zero Value)

Expected output:

Non-pointer: 42
Pointer: 42
Nil pointer valid: false

Impact

This change unblocks usage of packages that depend on reflect.Indirect, including:

  • encoding/binary
  • go/importer
  • And other standard library packages

Fixes #1354

🤖 Generated with codeagent

Implements reflect.Indirect function to support pointer dereferencing.
This function returns the value that a pointer points to, or returns
the value unchanged if it's not a pointer.

Fixes #1354

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <luoliwoshang@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Oct 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.18%. Comparing base (d2a2225) to head (a74ca94).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1355   +/-   ##
=======================================
  Coverage   90.18%   90.18%           
=======================================
  Files          43       43           
  Lines       12752    12752           
=======================================
  Hits        11501    11501           
  Misses       1092     1092           
  Partials      159      159           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

Choose a reason for hiding this comment

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

move to _demo folder & with compare test and use panic() for unexpected result @xgopilot

Copy link
Contributor Author

@xgopilot xgopilot bot Oct 17, 2025

Choose a reason for hiding this comment

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

✅ Task completed

I've successfully moved the test from _cmptest/reflect_indirect/ to _demo/go/reflect-indirect/ and refactored it to use panic() for validation.

Changes made:

  • Created new test at _demo/go/reflect-indirect/reflect-indirect.go
  • Replaced fmt.Println statements with proper assertions using panic()
  • Added validation that panics when tests fail
  • Added println("PASS") at the end to indicate success
  • Removed old test directory _cmptest/reflect_indirect/

Test validation:

The refactored test now:

  1. Non-pointer test: Validates value is 42, panics if not
  2. Pointer test: Validates dereferenced value is 42, panics if not
  3. Nil pointer test: Validates result is invalid, panics if valid
  4. Prints "PASS" when all assertions succeed

Committed and pushed to branch feature/reflect-indirect.


View workflow run | Branch: feature/reflect-indirect

- Moved test from _cmptest/reflect_indirect/ to _demo/go/reflect-indirect/
- Refactored to use panic() for validation instead of fmt.Println
- Added proper assertions for all test cases

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <luoliwoshang@users.noreply.github.com>
Copy link
Member

Choose a reason for hiding this comment

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

@xgopilot keep folder name style like other demo folder

Copy link
Contributor Author

@xgopilot xgopilot bot Oct 17, 2025

Choose a reason for hiding this comment

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

✅ Task completed

Renamed folder from reflect-indirect to reflectindirect to match the naming convention of other demo folders (e.g., reflectfunc, gotime, timedur).

Changes:

  • Renamed: _demo/go/reflect-indirect/_demo/go/reflectindirect/

Committed and pushed to branch feature/reflect-indirect.


View workflow run | Branch: feature/reflect-indirect

Copy link
Member

Choose a reason for hiding this comment

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

@xgopilot maybe we also need a struct case

Copy link
Contributor Author

@xgopilot xgopilot bot Oct 20, 2025

Choose a reason for hiding this comment

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

✅ Task completed

Added struct test cases to the reflect.Indirect demo as requested.

Changes made:

  • Added Test 4: Struct value test - validates that reflect.Indirect returns the same value when given a non-pointer struct
  • Added Test 5: Struct pointer test - validates that reflect.Indirect correctly dereferences a pointer to a struct

Test implementation:

Both tests use a Person struct with Name and Age fields and validate using panic() for assertion failures, following the existing pattern.

Committed and pushed to branch feature/reflect-indirect.


View workflow run | Branch: feature/reflect-indirect

xgopilot and others added 2 commits October 17, 2025 09:19
Follow naming convention of other demo folders (reflectfunc, gotime, etc.)

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <luoliwoshang@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <luoliwoshang@users.noreply.github.com>
@luoliwoshang
Copy link
Member

@xushiwei need review

@xushiwei xushiwei merged commit a2c8132 into main Oct 26, 2025
82 checks passed
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.

reflect support reflect.Indirect

3 participants