[Debugger] Box value-type keys when indexing IDictionary - #9274
dudikeleti wants to merge 3 commits into
Conversation
BenchmarksBenchmark execution time: 2026-09-19 00:53:10 Comparing candidate commit d4b1e49 in PR branch Found 0 performance improvements and 12 performance regressions! Performance is the same for 60 metrics, 0 unstable metrics, 71 known flaky benchmarks, 55 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9274) and master. ✅ No regressions detected |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d629ade64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
0d629ad to
32e242e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Missing value-type dictionary keys can fail during result conversion, and list indexing changes exceed the documented scope.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates debugger expression indexing to support value-type dictionary keys.
Changes:
- Boxes value-type keys for
IDictionaryindexing. - Adds dictionary and numeric list-index tests.
File summaries
| File | Description |
|---|---|
| tracer/test/Datadog.Trace.Tests/Debugger/DebuggerExpressionLanguageTests.cs | Adds index-expression regression tests. |
| tracer/src/Datadog.Trace/Debugger/Expressions/ProbeExpressionParser.Collection.cs | Converts index arguments for reflected indexers. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary of changes
ProbeExpressionParser.CallGetItemnow bindsIDictionary.get_Item(object)and boxes value-type keys soDictionary<int, int>[4]compiles and evaluates.intindexers, while rejecting narrowing conversions such asdoubleorlongtoint.Reason for change
test_expression_language_hash_operations(DEBUG-2602) failed on Hash5 index 4 withExpression of type System.Int32 cannot be used for parameter of type System.Object of method get_Item(System.Object).Dictionary<int, int>as non-genericIDictionaryand passed anintkey intoExpression.Callwithout boxing.Expression.Calldoes not automatically apply C# implicit argument conversions.Implementation details
get_Itemusingtypeof(object), matching theIDictionaryindexer signature.object.sbyte,byte,short,ushort, andcharlist/array indexes are converted toint.longindexes, remain rejected.Nullable<TValue>during lookup soIDictionary.get_Itemcan return null for a missing key without an unboxing exception.IDictionaryindexer rather thanIDictionary<TKey, TValue>: the generic indexer throwsKeyNotFoundExceptionfor missing keys, whileIDictionary.get_Itemreturns null. Switching would change existing missing-key probe results such asDictionaryKeyNotExist.Test coverage
DebuggerExpressionLanguageTestsDictionary<int, int>key returns its value.Dictionary<int, int>key returns null without an evaluation error.Other details
test_expression_language_hash_operationsin system-testsmanifests/dotnet.yml.