mirror of
https://github.com/esphome/esphome.git
synced 2026-06-02 03:02:19 +08:00
[core] Fix Pvariable placement new losing subclass identity (#15881)
This commit is contained in:
committed by
Jesse Hills
parent
06e5931ad7
commit
92cb6dd7fd
+33
-17
@@ -606,33 +606,43 @@ def Pvariable(id_: ID, rhs: SafeExpType, type_: "MockObj" = None) -> "MockObj":
|
|||||||
if isinstance(rhs, MockObj) and rhs.is_new_expr:
|
if isinstance(rhs, MockObj) and rhs.is_new_expr:
|
||||||
# For 'new' allocations, use placement new into static storage
|
# For 'new' allocations, use placement new into static storage
|
||||||
# to avoid heap fragmentation on embedded devices.
|
# 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
|
# 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"
|
storage_name = f"{component_ns}__{id_.id}__pstorage"
|
||||||
|
|
||||||
# Declare aligned byte array for the object storage
|
# Declare aligned byte array for the object storage
|
||||||
CORE.add_global(
|
CORE.add_global(
|
||||||
RawStatement(
|
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(
|
CORE.add_global(
|
||||||
AssignmentExpression(
|
AssignmentExpression(
|
||||||
f"static {the_type}",
|
f"static {pointer_type}",
|
||||||
"*const ",
|
"*const ",
|
||||||
id_,
|
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.
|
placement_new = CallExpression(f"new({id_.id}) {actual_type}", *call_expr.args)
|
||||||
# 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)
|
|
||||||
CORE.add(ExpressionStatement(placement_new))
|
CORE.add(ExpressionStatement(placement_new))
|
||||||
else:
|
else:
|
||||||
decl = VariableDeclarationExpression(id_.type, "*", id_, static=True)
|
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.
|
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.base = base
|
||||||
self.op = op
|
self.op = op
|
||||||
self.is_new_expr = is_new_expr
|
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":
|
def __getattr__(self, attr: str) -> "MockObj":
|
||||||
# prevent python dunder methods being replaced by mock objects
|
# prevent python dunder methods being replaced by mock objects
|
||||||
@@ -889,7 +903,9 @@ class MockObj(Expression):
|
|||||||
|
|
||||||
def __call__(self, *args: SafeExpType) -> "MockObj":
|
def __call__(self, *args: SafeExpType) -> "MockObj":
|
||||||
call = CallExpression(self.base, *args)
|
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):
|
def __str__(self):
|
||||||
return str(self.base)
|
return str(self.base)
|
||||||
@@ -903,7 +919,7 @@ class MockObj(Expression):
|
|||||||
|
|
||||||
@property
|
@property
|
||||||
def new(self) -> "MockObj":
|
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":
|
def template(self, *args: SafeExpType) -> "MockObj":
|
||||||
"""Apply template parameters to this object."""
|
"""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
|
||||||
Reference in New Issue
Block a user