From 9b19f31613986539a631bf950cc692f8f1bfd536 Mon Sep 17 00:00:00 2001 From: Skeets Norquist Date: Tue, 5 Sep 2023 16:16:29 -0700 Subject: [PATCH] Fix up some things per review feedback --- .vscode/cmake-variants.json | 8 +- CONTRIBUTING.rst | 4 +- docs/languages.rst | 15 +-- src/nunavut/cli/__init__.py | 1 + src/nunavut/cli/runners.py | 7 +- src/nunavut/lang/__init__.py | 11 +-- src/nunavut/lang/_config.py | 43 +++++++++ src/nunavut/lang/_language.py | 6 -- src/nunavut/lang/cpp/__init__.py | 40 +------- .../lang/cpp/templates/_composite_type.j2 | 92 +++++++++---------- src/nunavut/lang/properties.yaml | 2 +- test/gentest_arrays/test_arrays.py | 2 +- verification/CMakeLists.txt | 6 +- verification/cmake/utils.cmake | 2 +- ..._cetl++.cpp => test_array_cetl++14-17.cpp} | 0 15 files changed, 119 insertions(+), 120 deletions(-) rename verification/cpp/suite/{test_array_cetl++.cpp => test_array_cetl++14-17.cpp} (100%) diff --git a/.vscode/cmake-variants.json b/.vscode/cmake-variants.json index 0aa30932..47f8b5cb 100644 --- a/.vscode/cmake-variants.json +++ b/.vscode/cmake-variants.json @@ -58,11 +58,11 @@ } }, "CETL++": { - "short": "--std=cetl++", - "long": "Compile and link using the C++14 standard and use CETL.", + "short": "--std=cetl++14-17", + "long": "Compile and link using the C++14 standard and use CETL C++17 polyfill types.", "settings": { "NUNAVUT_VERIFICATION_LANG": "cpp", - "NUNAVUT_VERIFICATION_LANG_STANDARD": "cetl++" + "NUNAVUT_VERIFICATION_LANG_STANDARD": "cetl++14-17" } }, "C++17": { @@ -75,7 +75,7 @@ }, "C++17 PMR": { "short": "--std=c++17-pmr", - "long": "Compile and link using the C++17 standard and use polymorphic memory resources.", + "long": "Compile and link using the C++17 standard and use std polymorphic allocator.", "settings": { "NUNAVUT_VERIFICATION_LANG": "cpp", "NUNAVUT_VERIFICATION_LANG_STANDARD": "c++17-pmr" diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index f870fcc3..e1fbbed7 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -80,8 +80,8 @@ skip the docker invocations and use ``tox -s``. To run the language verification build you'll need to use a different docker container:: - docker pull ghcr.io/opencyphal/toolshed:ts20.4.1 - docker run --rm -it -v $PWD:/workspace ghcr.io/opencyphal/toolshed:ts20.4.1 + docker pull ghcr.io/opencyphal/toolshed:ts22.4.1 + docker run --rm -it -v $PWD:/workspace ghcr.io/opencyphal/toolshed:ts22.4.1 cd /workspace ./.github/verify.py -l c ./.github/verify.py -l cpp diff --git a/docs/languages.rst b/docs/languages.rst index 7be531f0..70214b1d 100644 --- a/docs/languages.rst +++ b/docs/languages.rst @@ -12,17 +12,18 @@ C++ (experimental) See :ref:`template-language-guide` until this section is more complete. -============================================== -Using a Different Variable-Length Array Type -============================================== +============================================================ +Using a Different Variable-Length Array Type and Allocator +============================================================ For now this tip is important for people using the experimental C++ support. To set which variable length array implementation to use, create a properties override yaml file and pass it to nnvg. Specifying the use of an allocator is optional (If ctor_convention is set to "default" then the allocator_include and allocator_type properties don't need to be set.) -Alternatively, you may specify the language standard argument as -std=c++17-pmr or -std=cetl++ as short-hand for -the following configurations shown below. +Alternatively, you may specify the language standard argument as -std=c++17-pmr or -std=cetl++14-17 as short-hand for +the following configurations shown below. Note that "cetl++14-17" means target C++14 but use the CETL C++17 polyfill +types. c++17-pmr.yaml """"""""""""""""" @@ -39,7 +40,7 @@ c++17-pmr.yaml allocator_is_default_constructible: true ctor_convention: "uses-trailing-allocator" -cetl++.yaml +cetl++14-17.yaml """"""""""""""""" .. code-block :: yaml @@ -59,7 +60,7 @@ nnvg command .. code-block :: bash - nnvg --configuration=c++17-pmr.yaml \ # or --configuration=cetl++.yaml + nnvg --configuration=c++17-pmr.yaml \ # or --configuration=cetl++14-17.yaml -l cpp \ --experimental-languages \ -I path/to/public_regulated_data_types/uavcan \ diff --git a/src/nunavut/cli/__init__.py b/src/nunavut/cli/__init__.py index e3acfc46..e09e19b7 100644 --- a/src/nunavut/cli/__init__.py +++ b/src/nunavut/cli/__init__.py @@ -484,6 +484,7 @@ def extension_type(raw_arg: str) -> str: ln_opt_group.add_argument( "--language-standard", "-std", + choices=["c11", "c++14", "cetl++14-17", "c++17", "c++17-pmr", "c++20"], help=textwrap.dedent( """ diff --git a/src/nunavut/cli/runners.py b/src/nunavut/cli/runners.py index 1cb2968a..37e0ce41 100644 --- a/src/nunavut/cli/runners.py +++ b/src/nunavut/cli/runners.py @@ -144,12 +144,6 @@ def _create_language_context(self) -> LanguageContext: language_options["enable_serialization_asserts"] = self._args.enable_serialization_asserts language_options["enable_override_variable_array_capacity"] = self._args.enable_override_variable_array_capacity if self._args.language_standard is not None: - valid_language_standards: typing.List[str] = ["c11", "c++14", "cetl++", "c++17", "c++17-pmr", "c++20"] - if self._args.language_standard not in valid_language_standards: - raise RuntimeError( - f"'{self._args.language_standard}' language standard not found. " - f"Must be one of {', '.join(valid_language_standards)}." - ) language_options["std"] = self._args.language_standard if self._args.configuration is None: @@ -167,6 +161,7 @@ def _create_language_context(self) -> LanguageContext: builder.set_target_language(target_language_name) builder.load_default_config(self._args.language_standard) builder.set_additional_config_files(additional_config_files) + builder.validate_langauge_options() builder.set_target_language_extension(self._args.output_extension) builder.set_target_language_configuration_override( Language.WKCV_NAMESPACE_FILE_STEM, self._args.namespace_output_stem diff --git a/src/nunavut/lang/__init__.py b/src/nunavut/lang/__init__.py index 32d10ed3..c4d51c98 100644 --- a/src/nunavut/lang/__init__.py +++ b/src/nunavut/lang/__init__.py @@ -19,8 +19,6 @@ logger = logging.getLogger(__name__) -NUNAVUT_LANG_CPP = "nunavut.lang.cpp" - class UnsupportedLanguageError(ValueError): """ @@ -223,11 +221,10 @@ def set_target_language(self, target_language: typing.Optional[str]) -> "Languag return self def load_default_config(self, language_standard: str) -> None: - self._ln_loader.config # Accessing this property causes the defaults to load - defaults_key = f"{language_standard}_options" - if defaults_key in self._ln_loader.config.sections()[NUNAVUT_LANG_CPP]: - defaults_data = self._ln_loader.config.get_config_value_as_dict(NUNAVUT_LANG_CPP, defaults_key) - self._ln_loader.config.update_section(NUNAVUT_LANG_CPP, {"options": defaults_data}) + self._ln_loader.config.apply_defaults(language_standard) + + def validate_langauge_options(self) -> None: + self._ln_loader.config.validate_language_options() def set_additional_config_files( self, additional_config_files: typing.List[pathlib.Path] diff --git a/src/nunavut/lang/_config.py b/src/nunavut/lang/_config.py index 38d2564a..0853fb51 100644 --- a/src/nunavut/lang/_config.py +++ b/src/nunavut/lang/_config.py @@ -10,10 +10,33 @@ import types import typing +from enum import auto, Enum + from yaml import Loader as YamlLoader from yaml import load as yaml_loader from nunavut._utilities import deep_update +NUNAVUT_LANG_CPP = "nunavut.lang.cpp" + + +class ConstructorConvention(Enum): + Default = "default" + UsesLeadingAllocator = "uses-leading-allocator" + UsesTrailingAllocator = "uses-trailing-allocator" + + @staticmethod + def parse_string(s: str) -> typing.Optional[typing.Any]: # annoying mypy cheat due to returning type being defined + for e in ConstructorConvention: + if s == e.value: + return e + return None + + +class SpecialMethod(Enum): + DefaultConstructorWithOptionalAllocator = auto() + CopyConstructorWithAllocator = auto() + MoveConstructorWithAllocator = auto() + class LanguageConfig: """ @@ -595,6 +618,26 @@ def get_config_value_as_list( return default_value + def apply_defaults(self, language_standard: str) -> None: + defaults_key = f"{language_standard}_options" + if defaults_key in self.sections()[NUNAVUT_LANG_CPP]: + defaults_data = self.get_config_value_as_dict(NUNAVUT_LANG_CPP, defaults_key) + self.update_section(NUNAVUT_LANG_CPP, {"options": defaults_data}) + + def validate_language_options(self) -> None: + options = self.get_config_value_as_dict(NUNAVUT_LANG_CPP, "options") + ctor_convention_str: str = options["ctor_convention"] + ctor_convention = ConstructorConvention.parse_string(ctor_convention_str) + if not ctor_convention: + raise RuntimeError( + f"ctor_convention property '{ctor_convention_str}' is invalid and must be one of " + + (",".join([f"'{e.value}'" for e in ConstructorConvention])) + ) + if ctor_convention != ConstructorConvention.Default and not options["allocator_type"]: + raise RuntimeError( + f"allocator_type property must be specified when ctor_convention is '{ctor_convention_str}'" + ) + # +-------------------------------------------------------------------------------------------------------------------+ # | VersionReader diff --git a/src/nunavut/lang/_language.py b/src/nunavut/lang/_language.py index 05cf96b8..0734e96f 100644 --- a/src/nunavut/lang/_language.py +++ b/src/nunavut/lang/_language.py @@ -118,12 +118,6 @@ def __init__(self, language_module_name: str, config: LanguageConfig, **kwargs: self._tests = dict() # type: typing.Dict[str, typing.Callable] self._uses = dict() # type: typing.Dict[str, typing.Callable] - self._validate_language_options(self._language_options) - - def _validate_language_options(self, language_options: typing.Mapping[str, typing.Any]) -> None: - """Subclasses may override this method to validate language-specific options""" - pass - def __getattr__(self, name: str) -> typing.Any: """ Any attribute access to a Language object will return the regular properties and diff --git a/src/nunavut/lang/cpp/__init__.py b/src/nunavut/lang/cpp/__init__.py index a58a869a..5c806ba7 100644 --- a/src/nunavut/lang/cpp/__init__.py +++ b/src/nunavut/lang/cpp/__init__.py @@ -18,8 +18,6 @@ import pydsdl -from enum import auto, Enum - from nunavut._dependencies import Dependencies from nunavut._templates import ( template_environment_list_filter, @@ -30,30 +28,12 @@ from nunavut._utilities import YesNoDefault from nunavut.jinja.environment import Environment from nunavut.lang._common import IncludeGenerator, TokenEncoder, UniqueNameGenerator +from nunavut.lang._config import ConstructorConvention, SpecialMethod from nunavut.lang._language import Language as BaseLanguage from nunavut.lang.c import _CFit from nunavut.lang.c import filter_literal as c_filter_literal -class ConstructorConvention(Enum): - Default = "default" - UsesLeadingAllocator = "uses-leading-allocator" - UsesTrailingAllocator = "uses-trailing-allocator" - - @staticmethod - def parse_string(s: str) -> typing.Optional[typing.Any]: # annoying mypy cheat due to returning type being defined - for e in ConstructorConvention: - if s == e.value: - return e - return None - - -class SpecialMethod(Enum): - DefaultConstructorWithOptionalAllocator = auto() - CopyConstructorWithAllocator = auto() - MoveConstructorWithAllocator = auto() - - class Language(BaseLanguage): """ Concrete, C++-specific :class:`nunavut.lang.Language` object. @@ -82,19 +62,6 @@ def _handle_stropping_or_encoding_failure( # we couldn't help after all. raise the pending error. raise pending_error - def _validate_language_options(self, language_options: typing.Mapping[str, typing.Any]) -> None: - ctor_convention_str: str = language_options["ctor_convention"] - ctor_convention = ConstructorConvention.parse_string(ctor_convention_str) - if not ctor_convention: - raise RuntimeError( - f"ctor_convention property '{ctor_convention_str}' is invalid and must be one of " - + (",".join([f"'{e.value}'" for e in ConstructorConvention])) - ) - if ctor_convention != ConstructorConvention.Default and not language_options["allocator_type"]: - raise RuntimeError( - f"allocator_type property must be specified when ctor_convention is '{ctor_convention_str}'" - ) - @functools.lru_cache(maxsize=None) def _get_token_encoder(self) -> TokenEncoder: """ @@ -999,6 +966,7 @@ def filter_value_initializer(language: Language, instance: pydsdl.Any, special_m Emit an initialization expression for a C++ special method. """ + value_initializer: str = "" if ( isinstance(instance.data_type, pydsdl.PrimitiveType) or isinstance(instance.data_type, pydsdl.ArrayType) @@ -1032,9 +1000,9 @@ def filter_value_initializer(language: Language, instance: pydsdl.Any, special_m if rhs: args.append(rhs) args = leading_args + args + trailing_args - return "{" + ", ".join(args) + "}" + value_initializer = "{" + ", ".join(args) + "}" - return "" + return value_initializer @template_language_filter(__name__) diff --git a/src/nunavut/lang/cpp/templates/_composite_type.j2 b/src/nunavut/lang/cpp/templates/_composite_type.j2 index 6c25542c..cee7359d 100644 --- a/src/nunavut/lang/cpp/templates/_composite_type.j2 +++ b/src/nunavut/lang/cpp/templates/_composite_type.j2 @@ -33,52 +33,6 @@ {% endif -%} struct {{composite_type|short_reference_name}} final { -{%- if options.ctor_convention != ConstructorConvention.Default.value %} - using allocator_type = {{ options.allocator_type }}; - - // Default constructor - {{composite_type|short_reference_name}}(const allocator_type& allocator - {%- if options.allocator_is_default_constructible %} = allocator_type(){% endif %}) - {%- if composite_type.fields_except_padding %} :{% endif %} - {%- for field in composite_type.fields_except_padding %} - {{ field | id }}{{ field | value_initializer(SpecialMethod.DefaultConstructorWithOptionalAllocator) }}{%if not loop.last %},{%endif %} - {%- endfor %} - { } - - // Destructor - ~{{composite_type|short_reference_name}}() = default; - - // Copy constructor - {{composite_type|short_reference_name}}(const {{composite_type|short_reference_name}}&) = default; - - // Copy constructor with allocator - {{composite_type|short_reference_name}}(const {{composite_type|short_reference_name}}& rhs, const allocator_type& allocator) - {%- if composite_type.fields_except_padding %} :{% endif %} - {%- for field in composite_type.fields_except_padding %} - {{ field | id }}{{ field | value_initializer(SpecialMethod.CopyConstructorWithAllocator) }}{%if not loop.last %},{%endif %} - {%- endfor %} - { - } - - // Move constructor - {{composite_type|short_reference_name}}({{composite_type|short_reference_name}}&&) = default; - - // Move constructor with allocator - {{composite_type|short_reference_name}}({{composite_type|short_reference_name}}&& rhs, const allocator_type& allocator) - {%- if composite_type.fields_except_padding %} :{% endif %} - {%- for field in composite_type.fields_except_padding %} - {{ field | id }}{{ field | value_initializer(SpecialMethod.MoveConstructorWithAllocator) }}{%if not loop.last %},{%endif %} - {%- endfor %} - { - } - - // Copy assignment - {{composite_type|short_reference_name}}& operator=(const {{composite_type|short_reference_name}}&) = default; - - // Move assignment - {{composite_type|short_reference_name}}& operator=({{composite_type|short_reference_name}}&&) = default; -{%- endif %} - struct _traits_ // The name is surrounded with underscores to avoid collisions with DSDL attributes. { _traits_() = delete; @@ -114,6 +68,9 @@ struct {{composite_type|short_reference_name}} final static_assert({# -#} ExtentBytes < (std::numeric_limits<{{ typename_unsigned_bit_length }}>::max() / 8U), {# -#} "This message is too large to be handled by the selected types"); +{% if options.ctor_convention != ConstructorConvention.Default.value %} + using allocator_type = {{ options.allocator_type }}; +{% endif %} {%- for field in composite_type.fields_except_padding %} {%- if loop.first %} struct TypeOf @@ -126,6 +83,49 @@ struct {{composite_type|short_reference_name}} final {%- endif %} {%- endfor %} }; +{% if options.ctor_convention != ConstructorConvention.Default.value %} + // Default constructor + {{composite_type|short_reference_name}}(const _traits_::allocator_type& allocator + {%- if options.allocator_is_default_constructible %} = _traits_::allocator_type(){% endif %}) + {%- if composite_type.fields_except_padding %} :{% endif %} + {%- for field in composite_type.fields_except_padding %} + {{ field | id }}{{ field | value_initializer(SpecialMethod.DefaultConstructorWithOptionalAllocator) }}{%if not loop.last %},{%endif %} + {%- endfor %} + { } + + // Destructor + ~{{composite_type|short_reference_name}}() = default; + + // Copy constructor + {{composite_type|short_reference_name}}(const {{composite_type|short_reference_name}}&) = default; + + // Copy constructor with allocator + {{composite_type|short_reference_name}}(const {{composite_type|short_reference_name}}& rhs, const _traits_::allocator_type& allocator) + {%- if composite_type.fields_except_padding %} :{% endif %} + {%- for field in composite_type.fields_except_padding %} + {{ field | id }}{{ field | value_initializer(SpecialMethod.CopyConstructorWithAllocator) }}{%if not loop.last %},{%endif %} + {%- endfor %} + { + } + + // Move constructor + {{composite_type|short_reference_name}}({{composite_type|short_reference_name}}&&) = default; + + // Move constructor with allocator + {{composite_type|short_reference_name}}({{composite_type|short_reference_name}}&& rhs, const _traits_::allocator_type& allocator) + {%- if composite_type.fields_except_padding %} :{% endif %} + {%- for field in composite_type.fields_except_padding %} + {{ field | id }}{{ field | value_initializer(SpecialMethod.MoveConstructorWithAllocator) }}{%if not loop.last %},{%endif %} + {%- endfor %} + { + } + + // Copy assignment + {{composite_type|short_reference_name}}& operator=(const {{composite_type|short_reference_name}}&) = default; + + // Move assignment + {{composite_type|short_reference_name}}& operator=({{composite_type|short_reference_name}}&&) = default; +{%- endif %} {%- for constant in composite_type.constants %} {% if loop.first %} diff --git a/src/nunavut/lang/properties.yaml b/src/nunavut/lang/properties.yaml index 7dcec101..e235c6c2 100644 --- a/src/nunavut/lang/properties.yaml +++ b/src/nunavut/lang/properties.yaml @@ -320,7 +320,7 @@ nunavut.lang.cpp: allocator_type: "" allocator_is_default_constructible: true ctor_convention: "default" - cetl++_options: + cetl++14-17_options: variable_array_type_include: '"cetl/variable_length_array.hpp"' variable_array_type_template: "cetl::VariableLengthArray<{TYPE}, {REBIND_ALLOCATOR}>" variable_array_type_constructor_args: "{MAX_SIZE}" diff --git a/test/gentest_arrays/test_arrays.py b/test/gentest_arrays/test_arrays.py index b7db2dc4..4df981e2 100644 --- a/test/gentest_arrays/test_arrays.py +++ b/test/gentest_arrays/test_arrays.py @@ -82,7 +82,7 @@ def test_var_array_override_cpp(gen_paths): # type: ignore gen_paths.find_outfile_in_namespace("radar.Phased", namespace), re.compile(r'^#include "scotec/alloc.hpp"$'), re.compile(r'^#include "scotec/array.hpp"$'), - re.compile(r".*\bconst allocator_type& allocator = allocator_type()"), + re.compile(r".*\bconst _traits_::allocator_type& allocator = _traits_::allocator_type()"), re.compile(r"\s*using *antennae_per_bank *= *scotec::TerribleArray::rebind_alloc>;\s*"), re.compile(r"\s*using *bank_normal_rads *= *std::array;\s*"), re.compile(r"\s*antennae_per_bank{std::allocator_arg, *allocator, *2677},\s*"), diff --git a/verification/CMakeLists.txt b/verification/CMakeLists.txt index 3920b7ee..2061f5a7 100644 --- a/verification/CMakeLists.txt +++ b/verification/CMakeLists.txt @@ -279,7 +279,7 @@ if(LOCAL_VERIFICATION_LANG STREQUAL "cpp") endif() -if(NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "cetl++" OR NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "c++17-pmr") +if(NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "cetl++14-17" OR NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "c++17-pmr") # # Generate serialization support headers with config matching the langauge standard # @@ -377,7 +377,7 @@ foreach(NATIVE_TEST ${NATIVE_TESTS_CPP}) get_filename_component(NATIVE_TEST_NAME ${NATIVE_TEST} NAME_WE) # Skip tests not relevant to the specified language standard - if((NATIVE_TEST_NAME STREQUAL "test_array_cetl++" AND NOT NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "cetl++") + if((NATIVE_TEST_NAME STREQUAL "test_array_cetl++14-17" AND NOT NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "cetl++14-17") OR (NATIVE_TEST_NAME STREQUAL "test_array_c++17-pmr" AND NOT NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "c++17-pmr")) continue() @@ -397,7 +397,7 @@ foreach(NATIVE_TEST ${NATIVE_TESTS_CPP}) # Link the library that was built for the specified language standard set(TEST_LINK_LIBS "") - if((NATIVE_TEST_NAME STREQUAL "test_array_cetl++" AND NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "cetl++") + if((NATIVE_TEST_NAME STREQUAL "test_array_cetl++14-17" AND NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "cetl++14-17") OR (NATIVE_TEST_NAME STREQUAL "test_array_c++17-pmr" AND NUNAVUT_VERIFICATION_LANG_STANDARD STREQUAL "c++17-pmr")) set(TEST_LINK_LIBS dsdl-test-array-with-allocator) diff --git a/verification/cmake/utils.cmake b/verification/cmake/utils.cmake index 5a73ae06..21e23305 100644 --- a/verification/cmake/utils.cmake +++ b/verification/cmake/utils.cmake @@ -34,7 +34,7 @@ function(replace_cxx_std ARG_OVERRIDE_CXX_STD ARG_REPLACE_INOUT) # Convert the Nunavut CLI language standard into the underlying C++ compiler language standard flag - if(ARG_OVERRIDE_CXX_STD STREQUAL "cetl++") + if(ARG_OVERRIDE_CXX_STD STREQUAL "cetl++14-17") set(ARG_OVERRIDE_CXX_STD "c++14") elseif(ARG_OVERRIDE_CXX_STD STREQUAL "c++17-pmr") set(ARG_OVERRIDE_CXX_STD "c++17") diff --git a/verification/cpp/suite/test_array_cetl++.cpp b/verification/cpp/suite/test_array_cetl++14-17.cpp similarity index 100% rename from verification/cpp/suite/test_array_cetl++.cpp rename to verification/cpp/suite/test_array_cetl++14-17.cpp