[core] Fix Pvariable placement new losing subclass identity (#15881)

This commit is contained in:
Jonathan Swoboda
2026-04-20 22:33:48 -04:00
committed by GitHub
parent 213ab312d2
commit a43ee15b56
4 changed files with 84 additions and 17 deletions
+33 -17
View File
@@ -606,33 +606,43 @@ def Pvariable(id_: ID, rhs: SafeExpType, type_: "MockObj" = None) -> "MockObj":
if isinstance(rhs, MockObj) and rhs.is_new_expr:
# For 'new' allocations, use placement new into static storage
# to avoid heap fragmentation on embedded devices.
the_type = id_.type
#
# Storage must be sized and aligned for the actual instantiated class,
# which may be a subclass of id_.type (e.g. `cv.declare_id(BaseClass)`
# combined with `SubClass.new()` — used by ili9xxx, waveshare_epaper,
# etc. to select a model-specific constructor). Using id_.type would
# run the base-class default constructor instead, silently losing any
# subclass initialization. Template args live on the CallExpression
# and are re-emitted below.
call_expr = rhs.base
assert isinstance(call_expr, CallExpression), (
f"Expected CallExpression for placement new, got {type(call_expr)}"
)
actual_type = rhs.new_type if rhs.new_type is not None else id_.type
if call_expr.template_args is not None:
actual_type = f"{actual_type}{call_expr.template_args}"
pointer_type = id_.type
# Extract component namespace from type for memory analysis attribution
component_ns = _extract_component_ns(str(the_type))
component_ns = _extract_component_ns(str(actual_type))
storage_name = f"{component_ns}__{id_.id}__pstorage"
# Declare aligned byte array for the object storage
CORE.add_global(
RawStatement(
f"alignas({the_type}) static unsigned char {storage_name}[sizeof({the_type})];"
f"alignas({actual_type}) static unsigned char {storage_name}[sizeof({actual_type})];"
)
)
# Pointer declaration uses id_.type to preserve the declared base-class
# pointer type for downstream callers (polymorphism through base ptr).
CORE.add_global(
AssignmentExpression(
f"static {the_type}",
f"static {pointer_type}",
"*const ",
id_,
MockObj(f"reinterpret_cast<{the_type} *>({storage_name})"),
MockObj(f"reinterpret_cast<{pointer_type} *>({storage_name})"),
)
)
# Extract args from the CallExpression and rebuild as placement new.
# Template args are already encoded in the_type (e.g. GlobalsComponent<int>),
# so we only pass the constructor args, not template_args.
call_expr = rhs.base
assert isinstance(call_expr, CallExpression), (
f"Expected CallExpression for placement new, got {type(call_expr)}"
)
placement_new = CallExpression(f"new({id_.id}) {the_type}", *call_expr.args)
placement_new = CallExpression(f"new({id_.id}) {actual_type}", *call_expr.args)
CORE.add(ExpressionStatement(placement_new))
else:
decl = VariableDeclarationExpression(id_.type, "*", id_, static=True)
@@ -869,12 +879,16 @@ class MockObj(Expression):
Mostly consists of magic methods that allow ESPHome's codegen syntax.
"""
__slots__ = ("base", "op", "is_new_expr")
__slots__ = ("base", "op", "is_new_expr", "new_type")
def __init__(self, base, op=".", is_new_expr=False) -> None:
def __init__(self, base, op=".", is_new_expr=False, new_type=None) -> None:
self.base = base
self.op = op
self.is_new_expr = is_new_expr
# For `is_new_expr=True` objects, `new_type` holds the class name being
# constructed (e.g. "ili9xxx::ILI9XXXST7789V"). Needed by Pvariable so
# placement new uses the actual subclass rather than id_.type.
self.new_type = new_type
def __getattr__(self, attr: str) -> "MockObj":
# prevent python dunder methods being replaced by mock objects
@@ -889,7 +903,9 @@ class MockObj(Expression):
def __call__(self, *args: SafeExpType) -> "MockObj":
call = CallExpression(self.base, *args)
return MockObj(call, self.op, is_new_expr=self.is_new_expr)
return MockObj(
call, self.op, is_new_expr=self.is_new_expr, new_type=self.new_type
)
def __str__(self):
return str(self.base)
@@ -903,7 +919,7 @@ class MockObj(Expression):
@property
def new(self) -> "MockObj":
return MockObj(f"new {self.base}", "->", is_new_expr=True)
return MockObj(f"new {self.base}", "->", is_new_expr=True, new_type=self.base)
def template(self, *args: SafeExpType) -> "MockObj":
"""Apply template parameters to this object."""
@@ -0,0 +1,20 @@
esphome:
name: test
esp32:
board: esp32dev
framework:
type: arduino
spi:
clk_pin: GPIO18
mosi_pin: GPIO23
display:
- platform: ili9xxx
id: tft_display
model: ST7789V
cs_pin: GPIO5
dc_pin: GPIO17
reset_pin: GPIO16
invert_colors: false
@@ -0,0 +1,31 @@
"""Tests for the ili9xxx component."""
from __future__ import annotations
from collections.abc import Callable
from pathlib import Path
def test_ili9xxx_placement_new_uses_model_subclass(
generate_main: Callable[[str | Path], str],
component_config_path: Callable[[str], Path],
) -> None:
"""Regression test for ili9xxx picking the right constructor under placement new.
ili9xxx declares the ID as the base ``ILI9XXXDisplay`` but constructs a
model-specific subclass (e.g. ``ILI9XXXST7789V``) via ``MODELS[...].new()``.
Pvariable must emit placement new for the subclass — otherwise the base
default constructor runs and the panel is left with a null init sequence
and 0x0 dimensions, producing a silent blank screen.
"""
main_cpp = generate_main(component_config_path("ili9xxx_test.yaml"))
# Storage is sized for the subclass so the full object fits.
assert "sizeof(ili9xxx::ILI9XXXST7789V)" in main_cpp
assert "alignas(ili9xxx::ILI9XXXST7789V)" in main_cpp
# Pointer is declared as the base type for polymorphism.
assert "static ili9xxx::ILI9XXXDisplay *const tft_display" in main_cpp
# Placement new runs the subclass constructor — this is the actual regression fix.
assert "new(tft_display) ili9xxx::ILI9XXXST7789V()" in main_cpp
# Base-class default constructor must NOT be used.
assert "new(tft_display) ili9xxx::ILI9XXXDisplay()" not in main_cpp