Fix for OpenCyphal#277 (OpenCyphal#278)

* Fix for OpenCyphal#277 This fixes, documents, and covers with test the ability to use std::vector instead of the built-in variable-length array type. Also getting a head-start on OpenCyphal#224 Also,also got rid of rstcheck as that project seems to be a hot mess. Too many false positives and major bugs not getting fixed. We’ll just have to manually test the docs using the tox -e docs target. * fixing broken doctest
edu-rossrobotics · Jan 31, 2023 · b3b6939 · b3b6939
1 parent 496e32c
commit b3b6939
Show file tree

Hide file tree

Showing 12 changed files with 170 additions and 24 deletions.
diff --git a/docs/languages.rst b/docs/languages.rst
@@ -5,3 +5,37 @@ Software Language Generation Guide
 .. note ::
     This is a placeholder for documentation this project owes you, the user, for how to integrate nnvg with build
     systems and how to tune and optimize source code generation for each supported language.
+
+*************************
+C++ (experimental)
+*************************
+
+See :ref:`template-language-guide` until this section is more complete.
+
+==============================================
+Using a Different Variable-Length Array Type
+==============================================
+
+For now this tip is important for people using the experimental C++ support. To use :code:`std::vector` instead of the
+minimal build-in :code:`variable_length_array` type create a properties override yaml file and pass it to nnvg.
+
+vector.yaml
+"""""""""""""""""
+
+.. code-block :: yaml
+
+    nunavut.lang.cpp:
+      options:
+        variable_array_type_include: <vector>
+        variable_array_type_template: std::vector<{TYPE}>
+
+nnvg command
+""""""""""""""""""
+
+.. code-block :: bash
+
+    nnvg --configuration=vector.yaml \
+         -l cpp \
+        --experimental-languages \
+        -I path/to/public_regulated_data_types/uavcan \
+        /path/to/my_types
diff --git a/docs/templates.rst b/docs/templates.rst
@@ -1,3 +1,5 @@
+.. _template-language-guide:
+
 ################################################
 Template Language Guide
 ################################################

diff --git a/src/nunavut/__init__.py b/src/nunavut/__init__.py
@@ -87,6 +87,7 @@
 from .lang import LanguageContextBuilder as LanguageContextBuilder
 from .lang import UnsupportedLanguageError as UnsupportedLanguageError
 from .lang._config import LanguageConfig as LanguageConfig
+from ._exceptions import InternalError as InternalError
 
 if _sys.version_info[:2] < (3, 5):  # pragma: no cover
     print("A newer version of Python is required", file=_sys.stderr)
@@ -109,6 +110,7 @@
     "TEMPLATE_SUFFIX",
     "UnsupportedLanguageError",
     "YesNoDefault",
+    "InternalError",
     "__author__",
     "__copyright__",
     "__email__",

diff --git a/src/nunavut/_exceptions.py b/src/nunavut/_exceptions.py
@@ -0,0 +1,18 @@
+#
+# Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# Copyright (C) 2023  OpenCyphal Development Team  <opencyphal.org>
+# This software is distributed under the terms of the MIT License.
+#
+"""
+Exception types thrown by the core library.
+
+"""
+
+
+class InternalError(RuntimeError):
+    """Internal, opaque error within Nunavut.
+
+    This exception is a "should never happen" exception. If caught you've probably hit a bug.
+    This is the only exception type within the library that can be use where no unit tests are covering the error
+    (i.e. pragma: no cover branches).
+    """
diff --git a/src/nunavut/_version.py b/src/nunavut/_version.py
@@ -8,7 +8,7 @@
 .. autodata:: __version__
 """
 
-__version__ = "2.0.1"
+__version__ = "2.0.2"
 __license__ = "MIT"
 __author__ = "OpenCyphal"
 __copyright__ = "Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. Copyright (c) 2022 OpenCyphal."

diff --git a/src/nunavut/lang/cpp/__init__.py b/src/nunavut/lang/cpp/__init__.py
@@ -19,6 +19,7 @@
 import pydsdl
 
 from nunavut._dependencies import Dependencies
+from nunavut._exceptions import InternalError as NunavutInternalError
 from nunavut._templates import (
     template_environment_list_filter,
     template_language_filter,
@@ -149,16 +150,20 @@ def _has_variant(self) -> bool:
         return self._standard_version() >= 17
 
     @functools.lru_cache()
-    def _get_variable_length_array_path(self) -> typing.Optional[pathlib.Path]:
+    def _get_variable_length_array_path(self) -> pathlib.Path:
         """
         Returns a releative path, suitable for include statements, to the built-in default Variable Length Array (VLA)
         support type.
+
+        :raises: NunavutInternalError if the library was missing the required support file.
         """
         for support_file in self.get_support_files(ResourceType.TYPE_SUPPORT):
             if support_file.stem == "variable_length_array":
                 return (pathlib.Path("/".join(self.support_namespace)) / support_file.name).with_suffix(self.extension)
 
-        return None
+        raise NunavutInternalError(
+            "variable_length_array file was not found in the c++ support files?!"
+        )  # pragma: no cover
 
     @functools.lru_cache()
     def _get_default_vla_template(self) -> str:
@@ -208,11 +213,14 @@ def do_includes_test(use_foobar, extension):
                 vla_header_name = "nunavut/support/variable_length_array{}".format(extension)
                 foobar_header_name = "foobar.h"
                 include_value = '' if not use_foobar else foobar_header_name
+                language_options = {
+                    "variable_array_type_include": include_value
+                }
 
                 lang_cpp = (
                     LanguageContextBuilder(include_experimental_languages=True)
                         .set_target_language("cpp")
-                        .set_target_language_configuration_override("variable_array_type_include", include_value)
+                        .set_target_language_configuration_override("options", language_options)
                         .set_target_language_configuration_override(Language.WKCV_DEFINITION_FILE_EXTENSION, extension)
                         .create()
                         .get_target_language()
@@ -246,6 +254,7 @@ def do_includes_test(use_foobar, extension):
             do_includes_test(False, ".h")
         """
         std_includes = []  # type: typing.List[str]
+        std_includes.append("limits")  # we always include limits to support static assertions
         if self.get_config_value_as_bool("use_standard_types"):
             if dep_types.uses_integer:
                 std_includes.append("cstdint")
@@ -257,15 +266,13 @@ def do_includes_test(use_foobar, extension):
 
         if dep_types.uses_variable_length_array:
             vla_include = None  # type: typing.Optional[str]
-            variable_array_include = self.get_config_value("variable_array_type_include", "")
-            if variable_array_include != "":
+            variable_array_include = str(self.get_option("variable_array_type_include", ""))
+            if len(variable_array_include) > 0:
                 vla_include = variable_array_include
             else:
-                vla_path = self._get_variable_length_array_path()
-                if vla_path is not None:
-                    vla_include = vla_path.as_posix()
+                vla_include = '"{}"'.format(self._get_variable_length_array_path().as_posix())
             if vla_include is not None:
-                includes_formatted.append('"{}"'.format(vla_include))
+                includes_formatted.append(vla_include)
 
         return includes_formatted
 
@@ -880,8 +887,8 @@ def filter_includes(
         # Listing the includes for a union with only integer types:
         template = "{% for include in my_type | includes -%}{{include}}{%- endfor %}"
 
-        # stdint.h will normally be generated
-        rendered = "<cstdint>"
+        # cstdint will normally be generated. limits is always generated.
+        rendered = "<cstdint><limits>"
 
     .. invisible-code-block: python
 
@@ -891,7 +898,7 @@ def filter_includes(
 
         # You can suppress std includes by setting use_standard_types to False under
         # nunavut.lang.cpp
-        rendered = ""
+        rendered = "<limits>"
 
     .. invisible-code-block: python
 

diff --git a/test/gentest_filters/test_filters.py b/test/gentest_filters/test_filters.py
@@ -224,7 +224,7 @@ def test_cpp_filter_includes(gen_paths, stropping, sort, mock_environment):  # t
 
     test_subject = next(filter(lambda type: (type.short_name == "hotness"), type_map))
     imports = filter_includes(lctx.get_target_language(), mock_environment, test_subject, sort=sort)
-    assert len(imports) == 5
+    assert len(imports) == 6
 
     def assert_path_in_imports(path: str) -> None:
         nonlocal imports
@@ -238,6 +238,7 @@ def assert_path_in_imports(path: str) -> None:
                 '"uavcan/time/SynchronizedTimestamp_1_0.h"',
                 "<array>",
                 "<cstdint>",
+                "<limits>",
             ] == imports
         else:
 
@@ -249,6 +250,7 @@ def assert_path_in_imports(path: str) -> None:
                     '"uavcan/str/bar_1_0.h"',
                     "<array>",
                     "<cstdint>",
+                    "<limits>",
                 ),
             )
     elif sort:
@@ -258,6 +260,7 @@ def assert_path_in_imports(path: str) -> None:
             '"uavcan/time/SynchronizedTimestamp_1_0.h"',
             "<array>",
             "<cstdint>",
+            "<limits>",
         ] == imports
     else:
         map(
@@ -268,6 +271,7 @@ def assert_path_in_imports(path: str) -> None:
                 '"uavcan/str/bar_1_0.h"',
                 "<array>",
                 "<cstdint>",
+                "<limits>",
             ),
         )
 

diff --git a/test/gentest_lang/test_lang.py b/test/gentest_lang/test_lang.py
@@ -432,3 +432,8 @@ def test_either_target_or_extension() -> None:
         .get_target_language()
         .name
     )
+
+def test_lang_cpp_use_vector(gen_paths) -> None:
+    """
+    Test override of the built-in variable-length array type.
+    """
diff --git a/test/gentest_versions/test_versions.py b/test/gentest_versions/test_versions.py
@@ -12,7 +12,7 @@
 
 from nunavut import build_namespace_tree
 from nunavut.jinja import DSDLCodeGenerator
-from nunavut.lang import Language, LanguageContextBuilder
+from nunavut.lang import LanguageContextBuilder
 
 
 include_pattern_map = (

diff --git a/test/gettest_properties/dsdl/proptest/hasvla.1.0.uavcan b/test/gettest_properties/dsdl/proptest/hasvla.1.0.uavcan
@@ -0,0 +1,3 @@
+uint8[<=50] vla_field
+
+@sealed
diff --git a/test/gettest_properties/test_properties.py b/test/gettest_properties/test_properties.py
@@ -0,0 +1,80 @@
+#
+# Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# Copyright (C) 2018-2020  OpenCyphal Development Team  <opencyphal.org>
+# This software is distributed under the terms of the MIT License.
+#
+
+import pathlib
+from pathlib import Path
+
+import pydsdl
+import pytest
+import yaml
+import re
+
+from nunavut import build_namespace_tree
+from nunavut.jinja import DSDLCodeGenerator
+from nunavut.lang import Language, LanguageClassLoader, LanguageContextBuilder
+
+vla_properties = (
+    ["", ""],
+    ["std::vector<{TYPE}>", "<vector>"],
+)
+
+
+@pytest.mark.parametrize("variable_array_type_template,variable_array_type_include", vla_properties)
+def test_issue_277(gen_paths, variable_array_type_template: str, variable_array_type_include: str):  # type: ignore
+    """
+    Writes a temporary yaml file to override configuration and verifies the values made it through to the generated
+    C++ output.
+    """
+
+    override_language_options = {
+        "variable_array_type_template": variable_array_type_template,
+        "variable_array_type_include": variable_array_type_include,
+    }
+
+    if variable_array_type_template != "":
+        vla_decl_pattern = re.compile(r"\b|^{}\B".format(variable_array_type_template.format(TYPE="std::uint8_t")))
+        vla_include_pattern = re.compile(r"\B#include\s+{}\B".format(variable_array_type_include))
+    else:
+        vla_decl_pattern = re.compile(r"\b|^nunavut::support::VariableLengthArray<std::uint8_t,\s*50>\B")
+        vla_include_pattern = re.compile(r"\B#include\s+\"nunavut/support/variable_length_array\.hpp\"\B")
+
+    overrides_file = gen_paths.out_dir / pathlib.Path("overrides_test_issue_277.yaml")
+
+    overrides_data = {
+        LanguageClassLoader.to_language_module_name("cpp"): {Language.WKCV_LANGUAGE_OPTIONS: override_language_options}
+    }
+
+    with open(overrides_file, "w") as overrides_handle:
+        yaml.dump(overrides_data, overrides_handle)
+
+    root_namespace = str(gen_paths.dsdl_dir / Path("proptest"))
+    compound_types = pydsdl.read_namespace(root_namespace, [], allow_unregulated_fixed_port_id=True)
+    language_context = (
+        LanguageContextBuilder(include_experimental_languages=True)
+        .set_target_language("cpp")
+        .set_additional_config_files([overrides_file])
+        .create()
+    )
+    namespace = build_namespace_tree(compound_types, root_namespace, gen_paths.out_dir, language_context)
+    generator = DSDLCodeGenerator(namespace)
+    generator.generate_all(omit_serialization_support=True)
+
+    # Now read back in and verify
+    outfile = gen_paths.find_outfile_in_namespace("proptest.hasvla", namespace)
+
+    assert outfile is not None
+
+    found_vla_decl = False
+    found_vla_include = False
+    with open(str(outfile), "r") as header_file:
+        for line in header_file:
+            if not found_vla_decl and vla_decl_pattern.search(line):
+                found_vla_decl = True
+            if not found_vla_include and vla_include_pattern.search(line):
+                found_vla_include = True
+
+    assert found_vla_decl
+    assert found_vla_include
diff --git a/tox.ini b/tox.ini
@@ -21,7 +21,6 @@ deps =
     autopep8
     rope
     isort
-    rstcheck[sphinx] >= 6.0.0
 
 # +---------------------------------------------------------------------------+
 # | CONFIGURATION
@@ -216,14 +215,6 @@ commands =
     flake8 --benchmark --tee --output-file={envtmpdir}/flake8.txt --filename=*.py --exclude=**/jinja2/*,**/markupsafe/* src
     black --check --line-length 120 --force-exclude '(/jinja2/|/markupsafe\/)' src
     doc8 {toxinidir}/docs
-    rstcheck --ignore-directives D001 \
-             --ignore-directives autofunction,automodule,argparse \
-             --ignore-substitutions ":wave:" \
-             -r \
-             {toxinidir}/docs \
-             {toxinidir}/CONTRIBUTING.rst \
-             {toxinidir}/README.rst \
-             {toxinidir}/LICENSE.rst
     mypy -m nunavut \
          -m nunavut.jinja \
          -p nunavut.lang \