Code size report:

Reference:  tests/ports/unix: Add tests for m_tracked_realloc. [18735dd]
Comparison: py/parse: Recognise const assignments with type hints. [merge of 41137f0]
  mpy-cross:   +32 +0.008% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:   +32 +0.004% standard
      stm32:   +24 +0.006% PYBV10
      esp32:   +32 +0.002% ESP32_GENERIC
     mimxrt:   +16 +0.004% TEENSY40
        rp2:   +24 +0.003% RPI_PICO_W
       samd:   +24 +0.009% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:   +30 +0.007% VIRT_RV32

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.46%. Comparing base (18735dd) to head (41137f0).
⚠️ Report is 1 commits behind head on master.

@@           Coverage Diff           @@
##           master   #17643   +/-   ##
=======================================
  Coverage   98.46%   98.46%           
=======================================
  Files         176      176           
  Lines       22800    22802    +2     
=======================================
+ Hits        22451    22453    +2     
  Misses        349      349           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Forgot to add tests... thank you, codecov! I've also had the chance to reword the commit message as it wasn't entirely clear. I'll push the lot once the current test run is over to avoid having to receive loads of cancelled run reports.

Anyway, I wonder how difficult it would be to have something like mpy-tool.py's disassembler available in the tests' running environment. This way the tests could also verify that, for a given bit of code, optimised bytecode was emitted instead of regular code sequences - that would make some sense for this specific PR at least.

Would it work to do this in the Python code instead?

_MY_CONST: int
_MY_CONST = const(123)

If it works, it might be worth it to save those 80+ bytes.

I've just tried, and it does indeed work - after all the type annotations are discarded anyway. Also, I've simplified the code and turned out the final overhead is bigger, go figure. I'll revert the PR to what it looked like then.

From an end user's point of view, having annotated variables exhibit the same behaviour as non-annotated ones is probably a reasonable expectation, isn't it?

Yes, a reasonable expectation. And the compiler can be disabled on super-constrained systems. So the code size isn't as much of a concern as I initially made it to be.

I've limited the type checking to int types, which I hope it's all that's needed.

Actually, const supports bool and tuple. And also now float! So the type annotation could be rather complicated if you have nested tuples etc.

So... the parser could just support any type annotation, ie any expression. Or just any identifier.

Anyway, I wonder how difficult it would be to have something like mpy-tool.py's disassembler available in the tests' running environment. This way the tests could also verify that, for a given bit of code, optimised bytecode was emitted instead of regular code sequences

There are tests for generated bytecode, in particular one for const generation. See tests/cmdline/cmd_showbc_const.py.

Actually, const supports bool and tuple. And also now float! So the type annotation could be rather complicated if you have nested tuples etc.

Indeed. Luckily I limited myself to just handle int types with this! I've tried to extend the const condition check to cover both MP_QSTR_int and MP_QSTR_float as the checked annotation type but, unless I did the test incorrectly, it didn't seem to work as expected.

I can revisit this at another time though, so both float and bool can be handled this way as well.

There are tests for generated bytecode, in particular one for const generation. See tests/cmdline/cmd_showbc_const.py.

Thanks, I didn't know this was possible. Although what I had in mind was more or less a MicroPython implementation of CPython's dis module so unrelated future changes in the compiled output would not break the test unless they affect const loads.

Why do we need to check the annotation value and limit it to only int? Can't we just remove && MP_PARSE_NODE_LEAF_ARG(((mp_parse_node_struct_t *)pn1)->nodes[0]) == MP_QSTR_int) and basically ignore the annotation to allow all const expressions with annotations?

@agatti what do you think about @dlech 's suggestion above, to just allow any annotation? That's needed because MicroPython's consts can be more than just int.

Gosh, I missed that - sorry! I'm all for it, let me update the PR right away then.

Thanks for updating, the code changes look good now.

Regarding the tests: it looks like you've copied two existing tests and added annotations. I think that's overkill and makes it harder to maintain the tests. Instead I suggest adding one new test called, eg, const_annotated.py which just tests annotations of various kinds, eg:

# Test type annotations in combination with const.

from micropython import const

_X0: bool = const(True)
_X1: int = const(123)
_X2: str = const("test")
_X3: tuple = const((1, 2))
_X4: tuple[bool, int] = const((True, 4))

(Keep in mind not to use float because not all targets support floats.)

Thanks for the test template. The previous iteration duplicated the original tests because they only dealt with integers in the first place, so that would fit with the initial reduced scope of these changes.

Anyway, I've added a missing const-able type (bytes) to your suggestion and removed the rest of the duplicated code.