Merge branch 'pmderodat/lkt-error-nodes' into 'master'

Lkt lowering: add checks for error nodes

See merge request eng/libadalang/langkit!857

Author: pmderodat

langkit/compile_context.py

@@ -12,13 +12,13 @@
 from __future__ import annotations
 
 from collections import defaultdict
-from contextlib import contextmanager
+from contextlib import AbstractContextManager, contextmanager
 from enum import Enum
 from functools import reduce
 import importlib
 import os
 from os import path
-from typing import (Any, Callable, ContextManager, Dict, List, Optional, Set,
+from typing import (Any, Callable, Dict, Iterator, List, Optional, Set,
                     TYPE_CHECKING, Tuple, Union, cast)
 
 from funcy import lzip
@@ -852,19 +852,30 @@ def actual_version(self) -> str:
     def actual_build_date(self) -> str:
         return self.build_date or "undefined"
 
-    def lkt_context(self, lkt_node: L.LktNode) -> ContextManager[None]:
+    @staticmethod
+    def lkt_context(
+        lkt_node: L.LktNode | None
+    ) -> AbstractContextManager[None]:
         """
         Context manager to set the diagnostic context to the given node.
 
         :param lkt_node: Node to use as a reference for this diagnostic
-            context.
+            context. If it is ``None``, leave the diagnostic context unchanged.
         """
-        # Invalid type passed here will fail much later and only if a
-        # check_source_language call fails. To ease debugging, check that
-        # "lkt_node" has the right type here.
-        assert isinstance(lkt_node, L.LktNode)
+        if lkt_node is None:
+            @contextmanager
+            def null_ctx_mgr() -> Iterator[None]:
+                yield
+
+            return null_ctx_mgr()
+
+        else:
+            # Invalid type passed here will fail much later and only if a
+            # check_source_language call fails. To ease debugging, check that
+            # "lkt_node" has the right type here.
+            assert isinstance(lkt_node, L.LktNode)
 
-        return diagnostic_context(Location.from_lkt_node(lkt_node))
+            return diagnostic_context(Location.from_lkt_node(lkt_node))
 
     @staticmethod
     def lkt_doc(full_decl):

langkit/compiled_types.py

@@ -1,13 +1,14 @@
 from __future__ import annotations
 
 from collections import OrderedDict
+from contextlib import AbstractContextManager
 from dataclasses import dataclass
 import difflib
 from itertools import count, takewhile
 import pipes
 from typing import (
-    Callable, ClassVar, ContextManager, Dict, Iterator, List, Optional as Opt,
-    Sequence, Set, TYPE_CHECKING, Tuple, Union, ValuesView
+    Callable, ClassVar, Dict, Iterator, List, Optional as Opt, Sequence, Set,
+    TYPE_CHECKING, Tuple, Union, ValuesView
 )
 
 from langkit import names
@@ -381,7 +382,7 @@ def uses_entity_info(self) -> bool:
         return self._uses_entity_info
 
     @property
-    def diagnostic_context(self) -> ContextManager[None]:
+    def diagnostic_context(self) -> AbstractContextManager[None]:
         return diagnostic_context(self.location)
 
     @property

langkit/dsl.py

@@ -1,9 +1,9 @@
 from __future__ import annotations
 
+from contextlib import AbstractContextManager
 import itertools
 from typing import (
-    Any, ContextManager, Dict, List, Optional as Opt, TYPE_CHECKING, Tuple,
-    Type, Union
+    Any, Dict, List, Optional as Opt, TYPE_CHECKING, Tuple, Type, Union
 )
 
 from langkit.compiled_types import (
@@ -49,7 +49,7 @@ def array(cls):
                        '{}.array'.format(cls._name.camel))
 
     @classmethod
-    def _diagnostic_context(cls) -> ContextManager[None]:
+    def _diagnostic_context(cls) -> AbstractContextManager[None]:
         return diagnostic_context(cls._location)
 
     @staticmethod

langkit/envs.py

@@ -8,10 +8,11 @@
 
 from __future__ import annotations
 
+from contextlib import AbstractContextManager
 from enum import Enum
 from funcy import lsplit_by
 from itertools import count
-from typing import ContextManager, Dict, List, Optional, Type, cast, overload
+from typing import Dict, List, Optional, Type, cast, overload
 
 from langkit import names
 from langkit.compile_context import CompileCtx, get_context
@@ -252,7 +253,7 @@ def count(cls: Type[EnvAction], sequence: List[EnvAction]) -> int:
         self.actions = self.pre_actions + self.post_actions
 
     @property
-    def diagnostic_context(self) -> ContextManager[None]:
+    def diagnostic_context(self) -> AbstractContextManager[None]:
         """
         Diagnostic context for env specs.
         """

langkit/expressions/structs.py

@@ -1,8 +1,8 @@
 from __future__ import annotations
 
+from contextlib import AbstractContextManager
 import inspect
-from typing import (Any as _Any, ContextManager, Dict, List, Optional,
-                    Sequence, Tuple)
+from typing import Any as _Any, Dict, List, Optional, Sequence, Tuple
 
 import funcy
 
@@ -861,7 +861,7 @@ def __init__(self,
         self.node_data: AbstractNodeData
 
     @property
-    def diagnostic_context(self) -> ContextManager[None]:
+    def diagnostic_context(self) -> AbstractContextManager[None]:
         return diagnostic_context(self.location)
 
     def resolve_field(self) -> AbstractNodeData:

langkit/lexer/__init__.py

@@ -1,10 +1,11 @@
 from __future__ import annotations
 
 from collections import defaultdict
+from contextlib import AbstractContextManager
 from itertools import count
 import re
-from typing import (Any, ContextManager, Dict, Iterator, List, Optional,
-                    Sequence, Set, TYPE_CHECKING, Tuple, Type, Union, cast)
+from typing import (Any, Dict, Iterator, List, Optional, Sequence, Set,
+                    TYPE_CHECKING, Tuple, Type, Union, cast)
 
 from langkit.compile_context import CompileCtx, get_context
 from langkit.diagnostics import (
@@ -321,7 +322,7 @@ def signature(self) -> tuple:
                 sorted(t.signature for t in self.tokens))
 
     @property
-    def diagnostic_context(self) -> ContextManager[None]:
+    def diagnostic_context(self) -> AbstractContextManager[None]:
         assert self.location is not None
         return diagnostic_context(self.location)
 

langkit/lkt_lowering.py

@@ -2241,6 +2241,28 @@ def check_trait(trait_ref: Optional[L.LktNode],
                 'Inheritting from an enum node is forbidden'
             )
 
+        with self.ctx.lkt_context(error_node_trait_ref):
+            # Determine whether this node is abstract. Remember that base enum
+            # node types are abstract (it is their derivations that are
+            # concrete).
+            is_abstract = (
+                not isinstance(annotations, NodeAnnotations)
+                or annotations.abstract
+            )
+            if is_abstract and is_error_node:
+                error("Error nodes cannot be abstract")
+
+            # Determine whether this node is synthetic
+            is_synthetic = annotations.synthetic
+            if is_synthetic and is_error_node:
+                error("Error nodes cannot be synthetic")
+
+            if base_type and base_type.is_list and is_error_node:
+                error("Error nodes cannot be lists")
+
+            if is_token_node and is_error_node:
+                error("Error nodes cannot be token nodes")
+
         # Lower fields. Regular nodes can hold all types of fields, but token
         # nodes and enum nodes can hold only user field and properties.
         allowed_field_types = (
@@ -2271,11 +2293,10 @@ def check_trait(trait_ref: Optional[L.LktNode],
             doc=self.ctx.lkt_doc(decl.parent),
             base=base_type,
             fields=fields,
-            is_abstract=(not isinstance(annotations, NodeAnnotations)
-                         or annotations.abstract),
+            is_abstract=is_abstract,
             is_token_node=is_token_node,
             is_error_node=is_error_node,
-            is_synthetic=annotations.synthetic,
+            is_synthetic=is_synthetic,
             has_abstract_list=annotations.has_abstract_list,
             is_enum_node=is_enum_node,
             is_bool_node=is_bool_node,
@@ -2292,6 +2313,18 @@ def check_trait(trait_ref: Optional[L.LktNode],
                 qualifier=annotations.qualifier
             )
 
+        # Reject non-null fields for error nodes. Non-null fields can come from
+        # this node's own declaration, or they can come from inheritance.
+        if is_error_node:
+            error_msg = "Error nodes can only have null fields"
+            for f in result.get_parse_fields(include_inherited=True):
+                if not (f.null or f.abstract):
+                    if f.struct != result:
+                        error(f"{error_msg}: {f.qualname} is not null")
+                    else:
+                        with f.diagnostic_context:
+                            error(error_msg)
+
         return result
 
     def create_enum_node_alternatives(

langkit/parsers.py

@@ -23,14 +23,14 @@
 from __future__ import annotations
 
 from collections import OrderedDict
-from contextlib import contextmanager
+from contextlib import AbstractContextManager, contextmanager
 import difflib
 from funcy import keep
 import inspect
 from itertools import count
 from typing import (
-    Any, Callable, ContextManager, Dict, Iterator, List as _List, Optional,
-    Sequence, Set, TYPE_CHECKING, Tuple, Type, Union
+    Any, Callable, Dict, Iterator, List as _List, Optional, Sequence, Set,
+    TYPE_CHECKING, Tuple, Type, Union
 )
 
 import funcy
@@ -270,7 +270,7 @@ def __init__(self,
         If we loaded a Lkt unit, mapping of all grammar rules it contains.
         """
 
-    def context(self) -> ContextManager[None]:
+    def context(self) -> AbstractContextManager[None]:
         return diagnostic_context(self.location)
 
     def _add_rule(self, name: str, parser: Parser, doc: str = "") -> None:
@@ -762,7 +762,7 @@ def set_location(self, location: Location) -> None:
             c.set_location(self.location)
 
     @property
-    def diagnostic_context(self) -> ContextManager[None]:
+    def diagnostic_context(self) -> AbstractContextManager[None]:
         """
         Helper that will return a diagnostic context manager with parameters
         set for the grammar definition.

testsuite/tests/grammar/invalid_error_node/common.lkt

@@ -0,0 +1,12 @@
+import lexer_example
+
+@with_lexer(foo_lexer)
+grammar foo_grammar {
+    @main_rule main_rule <- or(Example(@example) | skip(ErrorDecl))
+}
+
+@abstract class FooNode implements Node[FooNode] {
+}
+
+@has_abstract_list class Example : FooNode implements TokenNode {
+}

testsuite/tests/grammar/invalid_error_node/test.out

@@ -1,22 +1,37 @@
-== test_abstract ==
-test.py:XXX: error: Error nodes cannot be abstract
+== test_abstract.lkt ==
+test_abstract.lkt:3:52: error: Error nodes cannot be abstract
+3 | @abstract class BaseErrorDecl : FooNode implements ErrorNode {
+  |                                                    ^^^^^^^^^
 
-== test_bad_type ==
-test.py:XXX: error: The "error_node" field, when present, must contain a boolean
 
-== test_derived_not_error ==
-test.py:XXX: error: "error_node" annotation inconsistent with inherited node
+== test_error_and_list.lkt ==
+test_error_and_list.lkt:3:56: error: Error nodes cannot be lists
+3 | class ErrorDecl : ASTList[FooNode, Example] implements ErrorNode {
+  |                                                        ^^^^^^^^^
 
-== test_error_and_list ==
-test.py:XXX: error: Error nodes cannot also be lists
 
-== test_error_and_token ==
-test.py:XXX: error: Error nodes cannot also be token nodes
+== test_error_and_token.lkt ==
+test_error_and_token.lkt:3:38: error: Error nodes cannot be token nodes
+3 | class ErrorDecl : FooNode implements ErrorNode, TokenNode {
+  |                                      ^^^^^^^^^
 
-== test_nonnull_field ==
-test.py:XXX: error: Only null fields allowed here
 
-== test_synthetic ==
-test.py:XXX: error: Error nodes cannot be synthetic
+== test_nonnull_field.lkt ==
+test_nonnull_field.lkt:4:18: error: Error nodes can only have null fields
+4 |     @parse_field f: Example
+  |                  ^^^^^^^^^^
+
+
+== test_nonnull_inherited_field.lkt ==
+test_nonnull_inherited_field.lkt:7:1: error: Error nodes can only have null fields: BaseErrorDecl.f is not null
+7 | class ErrorDecl : BaseErrorDecl implements ErrorNode {
+  | ^
+
+
+== test_synthetic.lkt ==
+test_synthetic.lkt:3:49: error: Error nodes cannot be synthetic
+3 | @synthetic class ErrorDecl : FooNode implements ErrorNode {
+  |                                                 ^^^^^^^^^
+
 
 Done