Use field identifiers when code gen struct items #12222
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
Field identifiers were not being used. Instead, the order of field identifiers was used, with ordinal (0 to N-1) indexes. This is likely true of other code generated elements, but this PR only fixes it for struct items. This PR requires PR project-chip#326 in the third_party/zap/repo to function, as that PR actually includes the field identifier in the query results. Tested by running Matter cert tests, and also by inspecting the generated code for correctness.
pullapprove
bot
requested review from
anush-apple,
austinh0,
balducci-apple,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chulspro,
Damian-Nordic,
electrocucaracha,
emargolis,
franck-apple,
harimau-qirex,
hawk248,
holbrookt,
jelderton,
jepenven-silabs,
jmartinez-silabs,
LuDuda,
lzgrablic02,
mrjerryjohns,
msandstedt,
pan-apple,
sagar-apple,
saurabhst,
selissia,
tecimovic and
turon
tcarmelveilleux
approved these changes
Nov 24, 2021
|
PR #12222: Size comparison from 8bfdc13 to 175f2d3 Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
|
Updated submodule, ran cert tests, so good to go. (Not sure why it says there's a conflict in the zap repo...) |
|
Argh, looks like master changed while I was updating this PR, between my merging master and updating the submodule. |
|
fast track - basic change, but requires zap merging |
|
PR #12222: Size comparison from 15929eb to 6e0ebb6 Full report (9 builds for k32w, p6, qpg, telink)
|
Re-ran cert tests.
|
Updated the couple generated files that changed. Re-ran cert tests. Should be good now? |
|
PR #12222: Size comparison from 15929eb to 2950fb4 Increases (3 builds for esp32, linux, p6)
Decreases (1 build for esp32)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
jmartinez-silabs
approved these changes
Nov 25, 2021
|
Removed fast track: has sufficient checkmarks |
|
PR #12222: Size comparison from 9834f97 to 7d05851 Increases (2 builds for esp32, linux)
Decreases (1 build for esp32)
Full report (38 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
mlepage-google
deleted the
use-field-identifiers-when-code-gen-struct-items
branch
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.
Problem
Field identifiers were not being used. Instead, the order of field
identifiers was used, with ordinal (0 to N-1) indexes.
This is likely true of other code generated elements, but this PR only
fixes it for struct items.
Change overview
Generate struct field IDs code properly with actual fieldId rather than index.
This PR requires PR #326 in the third_party/zap/repo to function,
as that PR actually includes the field identifier in the query
results.
Testing
Tested by running Matter cert tests, and also by inspecting the
generated code for correctness.