Skip to content

[WIP] Add cetl::variant - #92

Merged
pavel-kirienko merged 88 commits into
mainfrom
issue86
Mar 16, 2024
Merged

pavel-kirienko merged 88 commits into
mainfrom
issue86

Conversation

@pavel-kirienko

@pavel-kirienko pavel-kirienko commented Feb 28, 2024 •

Copy link
Copy Markdown
Member

This is just an ordinary variant, nothing special here.

Other changes:

  • In CETLVaSt, move special member function policies from the optional test into a separate header to make them reusable for the variant test.
  • Extend the typelist tools in CETLVaSt.
  • Add standard type traits from C++17 (I needed them for the variant): conjunction_v etc.
  • Add the overloaded helper class, which is not found in the standard library but is commonly used with visitation. We can move it somewhere if its current place is found unsuitable.
  • Add type_traits_ext.hpp with type traits intended for internal use mostly.
  • Change the SonarQube invocation to run it against an exception-enabled build to provide full coverage information.
  • Doxygen:

@pavel-kirienko pavel-kirienko self-assigned this Feb 28, 2024
Base automatically changed from issue8 to main February 28, 2024 10:34
…because I am going to need that for the variant #verification #docs #sonar
…alueless_by_exception; now we need to slightly adjust the internals before moving further to eliminate certain redundancies
…RAM to compile, let's see how it goes #verification #docs #sonar
…he CI has run out of memory #verification #docs #sonar
@pavel-kirienko
pavel-kirienko marked this pull request as ready for review March 9, 2024 21:32
@pavel-kirienko
pavel-kirienko force-pushed the issue86 branch 2 times, most recently from 46ab9e1 to 6403161 Compare March 9, 2024 22:04
@pavel-kirienko

pavel-kirienko commented Mar 9, 2024 •

Copy link
Copy Markdown
Member Author

Well, the pull request is finished, except that the CI is still misbehaving. I have already split the tests into six translation units to manage the memory consumption by the compiler. I can split it further if needed but I am not yet confident that this is going to help. The new error from the CI is this:

Received request to deprovision: The request was cancelled by the remote provider

According to this, it may happen due to high CPU/memory utilization: actions/runner-images#7897

To reduce memory footprint of the compiler
@pavel-kirienko

pavel-kirienko commented Mar 10, 2024 •

Copy link
Copy Markdown
Member Author

@thirtytwobits I would like to add an exception for the coverage requirement for variant.hpp because currently it is computed per template instantiation, and the test suite is built to test a small fraction of branches per instantiation, which leads to a very low coverage estimate.

image

I'm not sure if it warrants an extension of the coverage exclusion list:

--define sonar.coverage.exclusions="cetlvast/suites/unittest/**/*,cetlvast/suites/docs/examples/**/*,**/sonar.cpp"

thirtytwobits
thirtytwobits previously approved these changes Mar 14, 2024

@thirtytwobits thirtytwobits left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved but could you add this example first:

cetlvast/suites/docs/examples/example_09_variant.cpp

/// @file
/// Example of using cetl::variant.
///
/// @copyright
/// Copyright (C) OpenCyphal Development Team  <opencyphal.org>
/// Copyright Amazon.com Inc. or its affiliates.
/// SPDX-License-Identifier: MIT
///
#include "cetl/pf17/variant.hpp"
#include <string>
#include <algorithm>
#include <cassert>
#include <iostream>
#include <string>

#include <gtest/gtest.h>

TEST(example_09_variant, basic_usage)
{
//! [example_09_basic_usage]
/// This example is taken directly from the [cppreference.com](https://en.cppreference.com/w/cpp/utility/variant)
/// documentation.
    cetl::pf17::variant<int, float> v, w;
    v = 42; // v contains int
    int i = cetl::pf17::get<int>(v);
    assert(42 == i); // succeeds
    w = cetl::pf17::get<int>(v);
    w = cetl::pf17::get<0>(v); // same effect as the previous line
    w = v; // same effect as the previous line

//  cetl::pf17::get<double>(v); // error: no double in [int, float]
//  cetl::pf17::get<3>(v);      // error: valid index values are 0 and 1

    float* result = cetl::pf17::get_if<float>(&w); // w contains int, not float: will return null
    std::cout << "get_if<float>(&w) => " << result << '\n';

    using namespace std::literals;

    cetl::pf17::variant<std::string> x("abc");
    // converting constructors work when unambiguous
    x = "def"; // converting assignment also works when unambiguous

    cetl::pf17::variant<std::string, void const*> y("abc");
    // casts to void const * when passed a char const *
    assert(cetl::pf17::holds_alternative<void const*>(y)); // succeeds
    y = "xyz"s;
    assert(cetl::pf17::holds_alternative<std::string>(y)); // succeeds
//! [example_09_basic_usage]
}

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
17.3% Coverage on New Code (required ≥ 90%)

See analysis details on SonarCloud

@pavel-kirienko
pavel-kirienko merged commit c82232f into main Mar 16, 2024
@pavel-kirienko
pavel-kirienko deleted the issue86 branch March 16, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants