[C#] fix parse failure for extensions with large field numbers#9591
Conversation
|
Interesting. Per spec the max extension number is 2^29 - 1, or 536,870,911. |
jskeet
left a comment
There was a problem hiding this comment.
I'm fine with both the test and the fix - but I'm not sure why the test is being added to the compatibility checks (which are usually about checking that changes don't break anything in previously-generated code).
| } | ||
| } | ||
|
|
||
| [Test] |
There was a problem hiding this comment.
Is there any reason you've added this test in here as well? We don't normally do that.
There was a problem hiding this comment.
Thank you for the review.
I didn't know where to add the test cases, so I added them to both files.
It seems unnecessary, so I'll delete this change.
| public static int GetTagFieldNumber(uint tag) | ||
| { | ||
| return (int) tag >> TagTypeBits; | ||
| return (int) (tag >> TagTypeBits); |
There was a problem hiding this comment.
Ouch, that was a subtle bug. Thanks for catching it.
|
Great, thanks. I'll squash and merge on green. |
|
This shouldn't affect the MacOS Ruby Release, which is the only target still running. Merging. |
Currently, it is failing to parse messages with an extension whose field number is large (> 0x10000000), e.g. https://github.com/google/mediapipe/blob/e6c19885c6d3c6f410c730952aeed2852790d306/mediapipe/calculators/tensor/tensors_to_detections_calculator.proto#L25) because the tag value is cast to
intbefore shifting right.