From 13807f2f06a5622add39a4c1960dfae581ae7d70 Mon Sep 17 00:00:00 2001 From: rturrado Date: Mon, 25 Nov 2024 15:18:51 +0100 Subject: [PATCH] Remove a couple of linter warnings: - One is due to a 'new' in gtest/gtest.h code. - The other seems to be a false positive. Update docs. Make CoreFunction code look like Instruction code. Fix Instruction::operator==. Pass linters. --- docs/dev-guide/dev-guide.md | 9 +++++++-- include/libqasm/v3x/core_function.hpp | 8 +------- src/v3x/core_function.cpp | 23 +++++++++++++---------- src/v3x/instruction.cpp | 5 ++++- test/v3x/cpp/integration_test.cpp | 5 ++++- 5 files changed, 29 insertions(+), 21 deletions(-) diff --git a/docs/dev-guide/dev-guide.md b/docs/dev-guide/dev-guide.md index 6a5c277e..e3ab57a9 100644 --- a/docs/dev-guide/dev-guide.md +++ b/docs/dev-guide/dev-guide.md @@ -119,7 +119,7 @@ Tests are disabled by default. To enable them, use `-c tools.build:skip_test=Fal Build and serve on `http://127.0.0.1:8000/`. ```shell -export PTYHONPATH=./scripts/python +export PYTHONPATH=./scripts/python mkdocs serve ``` @@ -137,12 +137,17 @@ Continuous Integration will fail if the files do not adhere to a series of forma It is recommended to run these linters before pushing any change: ```shell +conan build . -pr:a=conan/profiles/tests-release-gcc-linux-x64 -b missing python3 ./scripts/run_cpp_linters.py . ``` !!! note - The linters require `clang-format-18` and `clang-tidy-18` to be installed on the system. + The linters require: + + - `clang-format-18` and `clang-tidy-18` to be installed on the system, and + - a build folder containing a `compile_commands.json`. `run_cpp_linters.py` will use `build/Release` as default, + but a different folder can be passed as second argument to the script. ## Docker diff --git a/include/libqasm/v3x/core_function.hpp b/include/libqasm/v3x/core_function.hpp index e5030c38..52655d66 100644 --- a/include/libqasm/v3x/core_function.hpp +++ b/include/libqasm/v3x/core_function.hpp @@ -49,14 +49,8 @@ class CoreFunction : public tree::Base { * param_types is a shorthand type specification string as parsed by cqasm::types::from_spec(). * return_type is a shorthand type specification char as parsed by cqasm::types::from_spec(). */ - CoreFunction(std::string name, const std::string& param_types, const char return_type); - CoreFunction() = default; - ~CoreFunction() override = default; - CoreFunction(const CoreFunction& t) = default; - CoreFunction(CoreFunction&& t) noexcept = default; - CoreFunction& operator=(const CoreFunction& t) = default; - CoreFunction& operator=(CoreFunction&& t) noexcept = default; + CoreFunction(std::string name, const std::string& param_types, const char return_type); bool operator==(const CoreFunction& rhs) const; inline bool operator!=(const CoreFunction& rhs) const { return !(*this == rhs); } diff --git a/src/v3x/core_function.cpp b/src/v3x/core_function.cpp index c723fad0..7930f822 100644 --- a/src/v3x/core_function.cpp +++ b/src/v3x/core_function.cpp @@ -50,29 +50,32 @@ void serialize(const function::CoreFunctionRef& obj, ::tree::cbor::MapWriter& ma if (obj.empty()) { return; } - map.append_string("n", obj->name); - auto aw = map.append_array("pt"); + map.append_string("name", obj->name); + auto param_types_array = map.append_array("param_types"); for (const auto& t : obj->param_types) { - aw.append_binary(::tree::base::serialize(::tree::base::Maybe{ t.get_ptr() })); + param_types_array.append_binary(::tree::base::serialize(::tree::base::Maybe{ t.get_ptr() })); } - aw.close(); + param_types_array.close(); map.append_binary( - "rt", ::tree::base::serialize(::tree::base::Maybe{ obj->return_type.get_ptr() })); + "return_type", ::tree::base::serialize(::tree::base::Maybe{ obj->return_type.get_ptr() })); } template <> function::CoreFunctionRef deserialize(const ::tree::cbor::MapReader& map) { - if (!map.count("n")) { + // Remove what seems to be a false positive clang-analyzer-optin.cplusplus.VirtualCall + // NOLINTBEGIN + if (!map.count("name")) { return {}; } auto ret = tree::make(); - ret->name = map.at("n").as_string(); - auto ar = map.at("pt").as_array(); - for (const auto& element : ar) { + ret->name = map.at("name").as_string(); + auto param_types_array = map.at("param_types").as_array(); + for (const auto& element : param_types_array) { ret->param_types.add(::tree::base::deserialize(element.as_binary())); } - ret->return_type = ::tree::base::deserialize(map.at("rt").as_binary()); + ret->return_type = ::tree::base::deserialize(map.at("return_type").as_binary()); return ret; + // NOLINTEND } } // namespace cqasm::v3x::primitives diff --git a/src/v3x/instruction.cpp b/src/v3x/instruction.cpp index 39a7c00c..7b01a635 100644 --- a/src/v3x/instruction.cpp +++ b/src/v3x/instruction.cpp @@ -23,7 +23,7 @@ Instruction::Instruction(std::string name, const std::optional& ope * Equality operator. */ bool Instruction::operator==(const Instruction& rhs) const { - return name == rhs.name && operand_types == rhs.operand_types; + return name == rhs.name && operand_types.equals(rhs.operand_types); } /** @@ -59,6 +59,8 @@ void serialize(const instruction::InstructionRef& obj, ::tree::cbor::MapWriter& template <> instruction::InstructionRef deserialize(const ::tree::cbor::MapReader& map) { + // Remove what seems to be a false positive clang-analyzer-optin.cplusplus.VirtualCall + // NOLINTBEGIN if (!map.count("name")) { return {}; } @@ -69,6 +71,7 @@ instruction::InstructionRef deserialize(const ::tree::cbor::MapReader& map) { ret->operand_types.add(::tree::base::deserialize(element.as_binary())); } return ret; + // NOLINTEND } } // namespace cqasm::v3x::primitives diff --git a/test/v3x/cpp/integration_test.cpp b/test/v3x/cpp/integration_test.cpp index 85201982..1b7f247b 100644 --- a/test/v3x/cpp/integration_test.cpp +++ b/test/v3x/cpp/integration_test.cpp @@ -110,8 +110,11 @@ class IntegrationTest : public ::testing::Test { }; void register_tests() { + // Remove a clang-analyzer.cplusplus.NewDeleteLeaks warning in gtest/gtest.h + // NOLINTBEGIN cqasm::test::register_tests(fs::path{ "res" } / "v3x" / "tests" / "integration", - [=](fs::path test_path) -> IntegrationTest* { return new IntegrationTest(std::move(test_path)); }); + [=](fs::path test_path) -> IntegrationTest* { return new IntegrationTest{ std::move(test_path) }; }); + // NOLINTEND } } // namespace cqasm::v3x::test