Skip to content

Commit

Permalink
Fix name overlaps for lists in chip-tool. (#15119)
Browse files Browse the repository at this point in the history
Before this change, chip-tool's codegen just puts all the list buffers
on the stack and they all have to survive until we use them, so they
can't be in nested scopes.  They are suffixed with a nesting depth,
but if you have a list of structs that contain lists, the names for
the non-toplevel lists collide with each other.

This change switches over to the setup Darwin and Android already use:
heap-allocate the lists, hand the buffers to an RAII object higher up
on the stack that will free them when we are done, and then we can
scope each allocation and filling of a list in its own scope to limit
the visibility of the individual lists' names and avoid them colliding
with each other.
  • Loading branch information
bzbarsky-apple authored Feb 14, 2022
1 parent 7721423 commit d1c7e85
Show file tree
Hide file tree
Showing 4 changed files with 3,616 additions and 388 deletions.
2 changes: 2 additions & 0 deletions examples/chip-tool/templates/partials/test_cluster.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ class {{filename}}: public TestCommand
{{#if isCommand}}
using RequestType = chip::app::Clusters::{{asUpperCamelCase cluster}}::Commands::{{asUpperCamelCase command}}::Type;

ListFreer listFreer;
RequestType request;
{{#chip_tests_item_parameters}}
{{>commandValue ns=parent.cluster container=(concat "request." (asLowerCamelCase label)) definedValue=definedValue depth=0}}
Expand Down Expand Up @@ -334,6 +335,7 @@ class {{filename}}: public TestCommand
cluster.Associate({{>device}}, endpoint);
{{/if}}

ListFreer listFreer;
{{#chip_tests_item_parameters}}
{{zapTypeToEncodableClusterObjectType type ns=parent.cluster}} {{asLowerCamelCase name}}Argument;
{{>commandValue ns=parent.cluster container=(concat (asLowerCamelCase name) "Argument") definedValue=definedValue depth=0}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
Similarly, forceNotOptional=true and forceNotNullable=true because we
have accounted for those already. }}
{{#if definedValue.length}}
{{! This should really do heap-allocation with a function-scope-wide
auto-free setup, so we could guarantee no name collisions. }}
{{zapTypeToEncodableClusterObjectType type ns=ns forceNotList=true forceNotNullable=true forceNotOptional=true}} {{asLowerCamelCase label}}List_{{depth}}[{{definedValue.length}}];
{{#each definedValue}}
{{>commandValue ns=../ns container=(concat (asLowerCamelCase ../label) "List_" ../depth "[" @index "]") definedValue=. type=../type depth=(incrementDepth ../depth) parent=../parent}}
{{/each}}
{{container}} = {{asLowerCamelCase label}}List_{{depth}};
{
auto * listHolder_{{depth}} = new ListHolder<{{zapTypeToEncodableClusterObjectType type ns=ns forceNotList=true forceNotNullable=true forceNotOptional=true}}>({{definedValue.length}});
listFreer.add(listHolder_{{depth}});
{{#each definedValue}}
{{>commandValue ns=../ns container=(concat "listHolder_" ../depth "->mList[" @index "]") definedValue=. type=../type depth=(incrementDepth ../depth) parent=../parent}}
{{/each}}
{{container}} = chip::app::DataModel::List<{{zapTypeToEncodableClusterObjectType type ns=ns forceNotList=true forceNotNullable=true forceNotOptional=true}}>(listHolder_{{depth}}->mList, {{definedValue.length}});
}
{{else}}
{{container}} = chip::app::DataModel::List<{{zapTypeToEncodableClusterObjectType type ns=ns forceNotList=true forceNotNullable=true forceNotOptional=true}}>();
{{/if}}
Expand Down
1 change: 1 addition & 0 deletions examples/chip-tool/templates/tests-commands.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <commands/tests/TestCommand.h>
#include <commands/common/CommandInvoker.h>
#include <lib/core/Optional.h>
#include <lib/support/CHIPListUtils.h>
#include <system/SystemClock.h>

#include <math.h> // For INFINITY
Expand Down
Loading

0 comments on commit d1c7e85

Please sign in to comment.