Expose non-nullable elements from slices of pointers #2200
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bogdandrutu
force-pushed
the
notnilslice
branch
from
b8a3b1f
to
d2aa6cd
Compare
Codecov Report
@@ Coverage Diff @@
## master #2200 +/- ##
==========================================
+ Coverage 91.97% 92.15% +0.17%
==========================================
Files 271 271
Lines 15656 15269 -387
==========================================
- Hits 14400 14071 -329
+ Misses 854 824 -30
+ Partials 402 374 -28
Continue to review full report at Codecov.
|
bogdandrutu
force-pushed
the
notnilslice
branch
from
d2aa6cd
to
b27910d
Compare
bogdandrutu
changed the title
bogdandrutu
force-pushed
the
notnilslice
branch
6 times, most recently
from
0acc0f3
to
e0d2934
Compare
bogdandrutu
force-pushed
the
notnilslice
branch
from
e0d2934
to
81c3b9e
Compare
|
I do not quite understand the change. I see that you removed one level of indirection from the wrappers, but it is still not quite clear to me. Can you add a bit more details to the description? |
|
@tigrannajaryan PTAL, tried one more time :) |
bogdandrutu
force-pushed
the
notnilslice
branch
from
81c3b9e
to
72dd242
Compare
Currently for slices of pointers, pdata exposes elements that are pointers to the pointer in the slice (double pointers in the internal implementation). Because of this users have to deal with possible nil values, have to initialize elements (in some cases) etc, but in reality the elements in the slice cannot be nil, so this just adds extra unnecessary complexity. This is possible because: * Gogo proto (and protobuf) will not unmarshal any nil element in a slice; * Our APIs to add elements/remove elements from the slice will guarantee that we never have a nil element in the slice between [0, len-1]; This is an important change because will allow us to change the internal representation (use slice of pointers or non-pointers) without affecting the public API. Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
bogdandrutu
force-pushed
the
notnilslice
branch
from
72dd242
to
4eeb597
Compare
|
@tigrannajaryan PTAL |
tigrannajaryan
approved these changes
Dec 1, 2020
tigrannajaryan
added a commit
to tigrannajaryan/opentelemetry-collector-contrib
that referenced
this pull request
Dec 1, 2020
The changes are a result of removing IsNil, see open-telemetry/opentelemetry-collector#2200
Troels51
pushed a commit
to Troels51/opentelemetry-collector
that referenced
this pull request
Jul 5, 2024
…e but same timestamp (open-telemetry#2200)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently for slices of pointers, pdata exposes elements that are pointers to the pointer in the slice (double pointers in the internal implementation). Because of this users have to deal with possible
nilvalues, have to initialize elements (in some cases) etc, but in reality the elements in the slice cannot benil, so this just adds extra unnecessary complexity. This is possible because:ResizeorAppend) will guarantee that we never have a nil element in the slice between [0, len-1]; ForAppendby not allowing to create nil messages, we always append non-nil elements to the slice.This is an important change because will allow us to change the internal representation (use slice of pointers or non-pointers) without affecting the public API.
Signed-off-by: Bogdan Drutu bogdandrutu@gmail.com