Make contains/excludes constraints compile for lists of unsigned values. #19730
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
pullapprove
bot
requested review from
anush-apple,
arkq,
Byungjoo-Lee,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
erjiaqing,
franck-apple,
gjc13,
hawk248,
harsha-rajendran,
isiu-apple,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jtung-apple,
kghost,
lazarkov,
LuDuda,
mlepage-google,
msandstedt,
mspang and
robszewczyk
pullapprove
bot
requested review from
sagar-apple,
saurabhst,
selissia,
tcarmelveilleux,
tecimovic,
turon,
vijs,
wbschiller,
woody-apple,
xylophone21,
yufengwangca and
yunhanw-google
|
PR #19730: Size comparison from 147cab2 to 27b4b2b Increases (2 builds for cc13x2_26x2, telink)
Decreases (1 build for cc13x2_26x2)
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
bzbarsky-apple
force-pushed
the
fix-contains-unsigned
branch
from
27b4b2b to
66a7511
Compare
|
PR #19730: Size comparison from 2c7105c to 66a7511 Increases (3 builds for linux, telink)
Decreases (4 builds for cc13x2_26x2, cyw30739, telink)
Full report (39 builds for cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, nrfconnect, p6, telink)
|
Two changes here: 1) In chip_tests_iterate_expected_list, we were marking each individual value as an array (because "this.isArray" would be true in general when this helper is used). That caused asTypedLiteral to fail to map it to an integer basic type and hence we were not adding with the proper suffixes, and that lead to signed-to-unsigned comparison errors. This is the actual bugfix. 2) In asTypedLiteral, we really should be suffixing uint8_t with "U". It's not clear why compilers don't complain about signed-to-unsigned compares for that type the way they do for uint16/32/64_t, but conceptually this is the right thing to do. Fixes project-chip#19726
bzbarsky-apple
force-pushed
the
fix-contains-unsigned
branch
from
66a7511 to
b8535db
Compare
|
PR #19730: Size comparison from 2c7105c to b8535db Increases (4 builds for linux, nrfconnect, telink)
Decreases (3 builds for cyw30739, esp32, telink)
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
andy31415
approved these changes
Jun 21, 2022
jmeg-sfy
approved these changes
Jun 21, 2022
jmartinez-silabs
approved these changes
Jun 21, 2022
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.
Two changes here:
In chip_tests_iterate_expected_list, we were marking each individual value as
an array (because "this.isArray" would be true in general when this helper is
used). That caused asTypedLiteral to fail to map it to an integer basic type
and hence we were not adding with the proper suffixes, and that lead to
signed-to-unsigned comparison errors. This is the actual bugfix.
In asTypedLiteral, we really should be suffixing uint8_t with "U". It's not
clear why compilers don't complain about signed-to-unsigned compares for that
type the way they do for uint16/32/64_t, but conceptually this is the right
thing to do.
Fixes #19726
Problem
See #19726
Change overview
See above.
Testing
Checked that things compile with the YAML from #19726 and this change.