Skip to content

Conversation

@mishal
Copy link

@mishal mishal commented Jun 24, 2023

aiorepl: Fix usage of const function

@mishal mishal changed the title aiorepl: Fix usage of const method aiorepl: Fix usage of const function Jun 24, 2023
@jimmo
Copy link
Member

jimmo commented Jul 3, 2023

@mishal const is handled specially in MicroPython's parser. In fact, by adding micropython.const it prevents this from applying the const optimisation, it must be written as just plain const.

_A = const(1)
print(_A)

compiles to

00 LOAD_NAME print
02 LOAD_CONST_SMALL_INT 1
03 CALL_FUNCTION n=1 nkw=0
05 POP_TOP

whereas

import micropython
_A = micropython.const(1)
print(_A)

compiles to

06 LOAD_NAME micropython
08 LOAD_METHOD const
10 LOAD_CONST_SMALL_INT 1
11 CALL_METHOD n=1 nkw=0
13 STORE_NAME _A
15 LOAD_NAME print
17 LOAD_NAME _A
19 CALL_FUNCTION n=1 nkw=0
21 POP_TOP

You can see that _A is now a global, and also has to be loaded as the argument to print.

(I'm not actually sure why we provide micropython.const at all... seems like it's an easy way to catch people out. I will investigate).

@jimmo
Copy link
Member

jimmo commented Jul 4, 2023

See micropython/micropython#11929

@mishal
Copy link
Author

mishal commented Jul 14, 2023

OK, thanks for info. It looks like that almost every lib uses micropython.const to optimize the code, but no optimisation is really done.

@jimmo
Copy link
Member

jimmo commented Jul 17, 2023

It looks like that almost every lib uses micropython.const to optimize the code, but no optimisation is really done.

@mishal Could you clarify what you mean by "almost every lib"? Thanks

@mishal
Copy link
Author

mishal commented Jul 17, 2023

sorry...

I wanted to say, that (If I understand it correctly) lib authors trying to optimize their code using const (from micropython module) are not optimizing anything...

I use i2c eeprom in my project:

Also other authors use const from micropython module

see: https://github.com/search?q=%22from+micropython+import+const%22&type=code

The docs say that micropython.const is what should be used:

https://docs.micropython.org/en/latest/library/micropython.html?highlight=const#micropython.const

@jimmo
Copy link
Member

jimmo commented Jul 19, 2023

@mishal

The optimisation takes place if you use X = const(...), regardless of whether or not you do from micropython import const or not.

The only issue is if you write X = micropython.const(...) -- this will not do the optimisation.

Sorry I did not explain this very well -- and it's kind of confusing because these snippets are not the same, despite what regular Python semantics would tell you:

from micropython import const
X = const(1)  # Does the optimisation
import micropython
X = micropython.const(1)  # Does not do the optimisation

A third example:

from micropython import const as c
X = c(1)  # Does not do the optimisation

What's going on here is that the compiler specifically has a rule that detects the exact function name const. It doesn't know anything about the imports.

As far as I can see, both the examples linked from @peterhinch are doing the correct thing, and your github search doesn't show any issues -- doing from micropython import const is a valid and correct thing to do (see micropython/micropython#11929).

However, a different github search https://github.com/search?q=%22micropython.const%22&type=code shows many examples of the wrong thing.

The docs say that micropython.const is what should be used:

I don't think they do? They give a code example of just using plain const and also allude to the fact that this is special cased in the parser.
But it would probably be worthwhile explicitly saying "do not use micropython.const".

@mishal
Copy link
Author

mishal commented Aug 10, 2023

@jimmo thanks, now I get it!

This PR was about getting my IDE happy... there's no global const function in CPython. In your airepl.py you don't import the const from micropython module...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants