mirror of
https://github.com/esphome/esphome.git
synced 2026-06-02 03:02:19 +08:00
[core] Address Copilot review: robust brace depth, accurate docstrings
- Count { and } characters per line instead of matching whole-line
tokens. Current codegen only emits scope braces as standalone lines
(from cg.with_local_variable()), but the defensive change is robust
against future codegen emitting inline control flow like
`if (cond) {` or `} else {` on one line.
- Add a regression test covering those inline-brace patterns.
- Fix stale docstrings on ComponentMarker and cpp_main_section that
still claimed "stack frame released on return" and described the
IIFEs as "noinline". The IIFEs have no noinline attribute and rely
on scope-based lifetime shortening rather than guaranteed frames.
This commit is contained in:
+23
-12
@@ -565,11 +565,18 @@ def _wrap_in_iifes(lines: list[str], max_statements: int) -> list[str]:
|
|||||||
|
|
||||||
for line in lines:
|
for line in lines:
|
||||||
chunk.append(line)
|
chunk.append(line)
|
||||||
stripped = line.strip()
|
# Track brace depth by counting ``{`` and ``}`` characters per line
|
||||||
if stripped == "{":
|
# rather than matching whole-line tokens. Today the only codegen
|
||||||
depth += 1
|
# that emits scope braces as separate statements is
|
||||||
elif stripped == "}":
|
# ``cg.with_local_variable()`` (standalone ``{`` / ``}`` lines),
|
||||||
depth -= 1
|
# but counting is robust against future codegen that emits inline
|
||||||
|
# control-flow like ``if (cond) {`` or ``} else {`` on a single
|
||||||
|
# line. Multi-line statements (inline lambdas) carry balanced
|
||||||
|
# braces within one list entry so they contribute no net depth.
|
||||||
|
# Braces inside string literals would throw the count off, but
|
||||||
|
# esphome's generated main.cpp does not currently emit strings
|
||||||
|
# that contain unbalanced braces in main_statements.
|
||||||
|
depth += line.count("{") - line.count("}")
|
||||||
if depth == 0 and len(chunk) >= max_statements:
|
if depth == 0 and len(chunk) >= max_statements:
|
||||||
flush()
|
flush()
|
||||||
flush()
|
flush()
|
||||||
@@ -1069,13 +1076,17 @@ class EsphomeCore:
|
|||||||
if not components:
|
if not components:
|
||||||
return "\n".join(prefix) + "\n\n"
|
return "\n".join(prefix) + "\n\n"
|
||||||
|
|
||||||
# Each component's block is wrapped in IIFE lambdas so its stack
|
# Each component's block is wrapped in an IIFE lambda that
|
||||||
# frame is released on return, bounding peak stack during setup().
|
# introduces a nested scope, shortening the lifetimes of
|
||||||
# Large blocks are sub-split to cap single heavy components (e.g.
|
# temporaries so GCC can bound peak setup-time stack usage.
|
||||||
# sensor platforms with many filter registrations). "begin X" and
|
# The IIFE has no noinline attribute, so the compiler is free
|
||||||
# "end X" marker comments bracket the IIFE so the generated
|
# to inline the block when that produces smaller code without
|
||||||
# main.cpp is easy to scan by component; a comment-only component
|
# regressing peak stack. Large blocks are sub-split to cap
|
||||||
# gets a single "begin X" marker (no IIFE, no end marker).
|
# single heavy components (e.g. sensor platforms with many
|
||||||
|
# filter registrations). "begin X" and "end X" marker comments
|
||||||
|
# bracket the IIFE so the generated main.cpp is easy to scan by
|
||||||
|
# component; a comment-only component gets a single "begin X"
|
||||||
|
# marker (no IIFE, no end marker).
|
||||||
pieces = list(prefix)
|
pieces = list(prefix)
|
||||||
for name, body in components:
|
for name, body in components:
|
||||||
wrapped = _wrap_in_iifes(body, max_statements=50)
|
wrapped = _wrap_in_iifes(body, max_statements=50)
|
||||||
|
|||||||
@@ -437,7 +437,12 @@ class LineComment(Statement):
|
|||||||
class ComponentMarker(Statement):
|
class ComponentMarker(Statement):
|
||||||
"""Sentinel marker recorded in main_statements when a component's
|
"""Sentinel marker recorded in main_statements when a component's
|
||||||
to_code begins emitting code. ``cpp_main_section`` consumes these
|
to_code begins emitting code. ``cpp_main_section`` consumes these
|
||||||
to bracket each component's IIFE with begin/end comment markers."""
|
to bracket each component's generated block with begin/end comment
|
||||||
|
markers and to wrap it in an IIFE scope. The IIFE introduces a
|
||||||
|
nested scope so GCC can shorten temporary lifetimes and help reduce
|
||||||
|
peak setup-time stack usage; the lambda has no ``noinline``
|
||||||
|
attribute, so the compiler may still inline the block when that
|
||||||
|
produces smaller code without regressing peak stack."""
|
||||||
|
|
||||||
__slots__ = ("name",)
|
__slots__ = ("name",)
|
||||||
|
|
||||||
|
|||||||
@@ -934,6 +934,40 @@ def test_wrap_in_iifes_unbalanced_braces_fall_through() -> None:
|
|||||||
assert [line for line in result if line in lines] == lines
|
assert [line for line in result if line in lines] == lines
|
||||||
|
|
||||||
|
|
||||||
|
def test_wrap_in_iifes_never_splits_inline_brace_lines() -> None:
|
||||||
|
# Defensive: if codegen ever emits control flow with braces on the
|
||||||
|
# same line (if/else/for), the depth tracker should keep the whole
|
||||||
|
# scoped block together even with aggressive max_statements.
|
||||||
|
lines = [
|
||||||
|
"before();",
|
||||||
|
"if (cond) {",
|
||||||
|
"then_branch();",
|
||||||
|
"} else {",
|
||||||
|
"for (;;) {",
|
||||||
|
"loop_body();",
|
||||||
|
"}",
|
||||||
|
"}",
|
||||||
|
"after();",
|
||||||
|
]
|
||||||
|
assert core._wrap_in_iifes(lines, max_statements=1) == [
|
||||||
|
"[]() {",
|
||||||
|
"before();",
|
||||||
|
"}();",
|
||||||
|
"[]() {",
|
||||||
|
"if (cond) {",
|
||||||
|
"then_branch();",
|
||||||
|
"} else {",
|
||||||
|
"for (;;) {",
|
||||||
|
"loop_body();",
|
||||||
|
"}",
|
||||||
|
"}",
|
||||||
|
"}();",
|
||||||
|
"[]() {",
|
||||||
|
"after();",
|
||||||
|
"}();",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
def test_wrap_in_iifes_skips_comment_only_chunks() -> None:
|
def test_wrap_in_iifes_skips_comment_only_chunks() -> None:
|
||||||
# Components that emit only a ComponentMarker + config dump (no C++
|
# Components that emit only a ComponentMarker + config dump (no C++
|
||||||
# statements) should not be wrapped in an empty IIFE.
|
# statements) should not be wrapped in an empty IIFE.
|
||||||
|
|||||||
Reference in New Issue
Block a user