Progress on #91 #100
Progress on #91 #100
Conversation
|
I predict that the test I wrote for #99 will fail here because you seem to have encountered the same problem I had there. The culprit is where you added the extra check for none (can't link, am on mobile). The caching issue may appear to be fixed unless you have a test to validate it, I can't find such test here. Also, this is a 100-get ❤ |
…enCyphal#91 Disabling the Jinja cache results in the unique name generator state disappearing from the global Jinja context: > return _UniqueNameGenerator.ensure_generator_in_globals(env.globals)('c', adj_base_token, '_', '_') E TypeError: 'NoneType' object is not callable .tox/py35-test/lib/python3.5/site-packages/nunavut/lang/c.py:527: TypeError This issue affects all languages. The following test modification indicates that the state gets replaced with None: ------------------------- src/nunavut/lang/__init__.py ------------------------- index bd6524c..135a29a 100644 @@ -226,7 +226,7 @@ class _UniqueNameGenerator: def ensure_generator_in_globals(cls, environment_globals: typing.Dict[str, typing.Any]) -> '_UniqueNameGenerator': from .. import TypeLocalGlobalKey - if TypeLocalGlobalKey not in environment_globals: + if environment_globals.get(TypeLocalGlobalKey) is None: environment_globals[TypeLocalGlobalKey] = cls() return typing.cast('_UniqueNameGenerator', environment_globals[TypeLocalGlobalKey]) Am stuck. Progress on OpenCyphal#91 1) Fixing a problem with unique identifier generation (Issue OpenCyphal#88) 2) Generating C++ data structures (Issue OpenCyphal#91) Still TODO for Issue OpenCyphal#91 is serialization for types.
thirtytwobits
force-pushed
the
feature/91
branch
from
853b3bb to
5c7a66f
Compare
|
Let me know if this works for you. I've provided an example of using a template include instead of a macro to avoid the issues you are seeing with the unique identifiers. This includes a rebase with your change and then a modification of it. |
(quote from #99) Macros are allowed to have side effects. They work well in the current PyUAVCAN master where I keep the state in a template variable: For some reason, they break only when the global state is added from the side of the Python API. Switching to includes instead of macros is worse than the current solution so it's probably best to keep it unchanged then. I think we are misusing Jinja, it's getting in the way again. |
|
Maybe we should instead keep the state somewhere inside Nunavut instead of Jinja's globals so that it's not destroyed when a macro is invoked? |
|
Okay. This is the best I can figure out. There may be a way to create a "runtime macro" extension but that's out of scope for this change. |
|
Now the counter is not reset when a new data type starts being generated. The actual caching problem is not solved unless we use the more verbose macro invocation form. I suggest we go back to the old hack, somehow it seems less hacky that the new solution. |
|
Going back to the old solution means removal of the unique ID filter from Nunavut, |
Oh, I can reintroduce resetting the counter for each type but why should that matter? |
|
Because the output code for a type X should not be dependent on that of the type Y, otherwise it complicates tracking the changes, and in compiled languages it may lead to unnecessary recompilation. |
thirtytwobits
force-pushed
the
feature/91
branch
2 times, most recently
from
cd334fe to
c41bfd9
Compare
thirtytwobits
force-pushed
the
feature/91
branch
from
c41bfd9 to
7fd2ef1
Compare
|
Found a fix! |
Still TODO for Issue #91 is serialization for types.