-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add -Wconversion to various things in src/app #7902
Add -Wconversion to various things in src/app #7902
Conversation
Size increase report for "nrfconnect-example-build" from 336533f
Full report output
|
f57a407
to
7bad07e
Compare
@@ -176,13 +176,15 @@ uint8_t emberAfGetDynamicIndexFromEndpoint(EndpointId id) | |||
EmberAfStatus emberAfSetDynamicEndpoint(uint8_t index, EndpointId id, EmberAfEndpointType * ep, uint16_t deviceId, | |||
uint8_t deviceVersion) | |||
{ | |||
index += FIXED_ENDPOINT_COUNT; | |||
auto realIndex = index + FIXED_ENDPOINT_COUNT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto -> int?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could do that, but the point is we don't really care what this gets promoted to by C's rules. We just check that it's in-range and then cast it to the type we really want... So I was pretty carefully avoiding using int
there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not that we care or don't care. It's that the type is clear and it's "int". It isn't going to change and there aren't any other promotion rules like that. Why obfuscate?
7bad07e
to
6de1f2a
Compare
6de1f2a
to
478352e
Compare
Size increase report for "esp32-example-build" from 336533f
Full report output
|
Size increase report for "gn_qpg6100-example-build" from 336533f
Full report output
|
@Damian-Nordic @jmartinez-silabs Please take a look? |
* Add -Wconversion to various things in src/app. * Regenerate generated files
Problem
We are not using
-Wconversion
to build things insrc/app
, so we won't catch problems as we start changing the sizes of things like cluster/endpoint/attribute ids.Change overview
Add
-Wconversion
to some of the relevant .gn/.gni files, fix resulting problems.Testing
Compiled the code on esp32, Mac, nrfconnect.