From c174e5c70377333887da6b6116df63051dbcdbd5 Mon Sep 17 00:00:00 2001 From: Pierre-Marie de Rodat Date: Fri, 24 Jun 2022 09:38:58 +0000 Subject: [PATCH] Lkt lowering: fix lowering order for list node derivation Depending on the order picked by Langkit for node type lowering, processing the definition of a derived list node could crash. The problem was that lowering code just assumed so far that the root node was lowered before list nodes, and that list element nodes were lowered before the corresponding list nodes as well, whereas this has never been guaranteed. To fix this, we now force the lowering of the element node when lowering a list node, and stop assuming that the root node is lowered first. For #622 --- langkit/lkt_lowering.py | 67 ++++++++++++++----- .../tests/grammar/list_forward_decl/test.lkt | 19 ++++++ .../tests/grammar/list_forward_decl/test.out | 4 ++ .../tests/grammar/list_forward_decl/test.yaml | 1 + 4 files changed, 74 insertions(+), 17 deletions(-) create mode 100644 testsuite/tests/grammar/list_forward_decl/test.lkt create mode 100644 testsuite/tests/grammar/list_forward_decl/test.out create mode 100644 testsuite/tests/grammar/list_forward_decl/test.yaml diff --git a/langkit/lkt_lowering.py b/langkit/lkt_lowering.py index e3fcbd28d..2b6614d4f 100644 --- a/langkit/lkt_lowering.py +++ b/langkit/lkt_lowering.py @@ -1280,11 +1280,15 @@ def get_field(decl: L.NamedTypeDecl, name: str) -> L.Decl: assert isinstance(decl.f_decl, L.TypeDecl) self.lower_type_decl(decl.f_decl) - def resolve_type_decl(self, decl: L.TypeDecl) -> CompiledTypeOrDefer: + def resolve_type_decl(self, + decl: L.TypeDecl, + force_lowering: bool = False) -> CompiledTypeOrDefer: """ Fetch the CompiledType instance corresponding to the given type - declaration. If it's not lowered yet, return an appropriate - TypeRepo.Defer instance instead. + declaration. + + When ``force_lowering`` is ``False``, if ``decl`` is not lowered yet, + return an appropriate TypeRepo.Defer instance instead. :param decl: Lkt type declaration to resolve. """ @@ -1295,29 +1299,49 @@ def resolve_type_decl(self, decl: L.TypeDecl) -> CompiledTypeOrDefer: if result is not None: return result - # Not found: look now for an existing TypeRepo.Defer instance - result = self.type_refs.get(decl) - if result is not None: - return result + # Not found: unless lowering is forced, look now for an existing + # TypeRepo.Defer instance. + if not force_lowering: + result = self.type_refs.get(decl) + if result is not None: + return result - # Not found neither: create the TypeRepo.Defer instance + # If this is an instantiated generic type, try to build the + # corresponding CompiledType from the type actuals. if isinstance(decl, L.InstantiatedGenericType): inner_type = decl.p_get_inner_type actuals = decl.p_get_actuals if inner_type == self.array_type: assert len(actuals) == 1 - result = self.resolve_type_decl(actuals[0]).array + result = self.resolve_type_decl( + actuals[0], force_lowering + ).array elif inner_type == self.iterator_trait: assert len(actuals) == 1 - result = self.resolve_type_decl(actuals[0]).iterator + result = self.resolve_type_decl( + actuals[0], force_lowering + ).iterator elif inner_type == self.astlist_type: assert len(actuals) == 2 - root_node = self.resolve_type_decl(actuals[0]) - node = self.resolve_type_decl(actuals[1]) - assert root_node == T.root_node + root_node = actuals[0] + node = self.resolve_type_decl( + actuals[1], force_lowering=force_lowering + ) + + # Make sure that "root_node" is indeed a root node (a class + # with no base type). Lkt type checking as already supposed to + # make sure that "node" is a class (i.e. a node), and lowering + # already checks that there is exactly one node types + # hierarchy. + check_source_language( + isinstance(root_node, L.ClassDecl) + and root_node.f_syn_base_type is None, + "In ASTList[N1, N2], N1 must be the root node" + ) + assert isinstance(node, (ASTNodeType, TypeRepo.Defer)) result = node.list @@ -1330,11 +1354,18 @@ def resolve_type_decl(self, decl: L.TypeDecl) -> CompiledTypeOrDefer: .format(inner_type, decl) ) + # Otherwise, "decl" is not lowered yet: create a Defer object or lower + # it depending on "force_lowering". else: assert isinstance(decl, L.NamedTypeDecl) - result = getattr(T, decl.f_syn_name.text) + result = ( + self.lower_type_decl(decl) + if force_lowering else + getattr(T, decl.f_syn_name.text) + ) if isinstance(result, TypeRepo.Defer): + assert not force_lowering self.type_refs[decl] = result else: assert isinstance(result, CompiledType) @@ -1376,9 +1407,11 @@ def lower_type_decl(self, decl: L.TypeDecl) -> CompiledType: if isinstance(decl, L.InstantiatedGenericType): # At this stage, the only generic types should come from the # prelude (Array, ASTList), so there is no need to do anything - # special for them: just let the resolution mechanism build the - # CompiledType through CompiledType attribute access magic. - result = resolve_type(self.resolve_type_decl(decl)) + # special for them. However the type actuals must be lowered so + # that we can return a compiled type, and not a Defer object. + resolved = self.resolve_type_decl(decl, force_lowering=True) + assert isinstance(resolved, CompiledType) + result = resolved else: full_decl = decl.parent assert isinstance(full_decl, L.FullDecl) diff --git a/testsuite/tests/grammar/list_forward_decl/test.lkt b/testsuite/tests/grammar/list_forward_decl/test.lkt new file mode 100644 index 000000000..a12448f63 --- /dev/null +++ b/testsuite/tests/grammar/list_forward_decl/test.lkt @@ -0,0 +1,19 @@ +# Check that compiling a list node derivation that occurs in the Lkt source +# before the root node declaration works correctly (it used to crash). + +import lexer_example + +@with_lexer(foo_lexer) +grammar foo_grammar { + @main_rule main_rule <- AExampleList*(BExample("example")) +} + +class AExampleList : ASTList[CFooNode, BExample] { +} + +@has_abstract_list class BExample : CFooNode implements TokenNode { +} + +@abstract class CFooNode implements Node[CFooNode] { +} + diff --git a/testsuite/tests/grammar/list_forward_decl/test.out b/testsuite/tests/grammar/list_forward_decl/test.out new file mode 100644 index 000000000..6ed28905b --- /dev/null +++ b/testsuite/tests/grammar/list_forward_decl/test.out @@ -0,0 +1,4 @@ +== test.lkt == +Code generation was successful + +Done diff --git a/testsuite/tests/grammar/list_forward_decl/test.yaml b/testsuite/tests/grammar/list_forward_decl/test.yaml new file mode 100644 index 000000000..4a499fbd3 --- /dev/null +++ b/testsuite/tests/grammar/list_forward_decl/test.yaml @@ -0,0 +1 @@ +driver: lkt_compile