Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FIX] don't hard-fail on concept check #1394

Merged
merged 1 commit into from
Dec 9, 2019
+46 −44
Merged

[FIX] don't hard-fail on concept check #1394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ If possible, provide tooling that performs the changes, e.g. a shell-script.
* **Changed class signature of input/output file validators:**
Most user code will be unaffected; to fix possible compiler errors you need to add an empty template parameter list to
the respective instances (e.g. change `input_file_validator` to `input_file_validator<>`).
* The member type that denotes which arguments a `validator` can validate has been renamed from `value_type` to
Copy link
Contributor

Choose a reason for hiding this comment

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

a) why and b) what has this to do with the hard fails on concept checks?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

Ok, a different "fix" was necessary, because the problem is that the concept check of GCC doesn't cope with something existing, but being private/protected. It will result in hard errors instead of failing the concept.

Copy link
Member Author

Choose a reason for hiding this comment

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

(our ranges often have protected or private ::value_type)

`option_value_type`.

#### Core

Expand Down
4 changes: 2 additions & 2 deletions doc/tutorial/concepts/custom_validator_solution1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

struct custom_validator
{
using value_type = double; // used for all arithmetic types
using option_value_type = double; // used for all arithmetic types

void operator() (value_type const &) const
void operator() (option_value_type const &) const
{
// add implementation later
}
Expand Down
4 changes: 2 additions & 2 deletions doc/tutorial/concepts/custom_validator_solution2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

struct custom_validator
{
using value_type = double; // used for all arithmetic types
using option_value_type = double; // used for all arithmetic types

void operator() (value_type const & val) const
void operator() (option_value_type const & val) const
{
if ((std::round(val) != val) || // not an integer
(std::pow(std::round(std::sqrt(val)), 2) != val)) // not a square
Expand Down
4 changes: 2 additions & 2 deletions include/seqan3/argument_parser/argument_parser.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class argument_parser
template <typename option_type, validator validator_type = detail::default_validator<option_type>>
//!\cond
requires (argument_parser_compatible_option<option_type> ||
argument_parser_compatible_option<typename option_type::value_type>) &&
argument_parser_compatible_option<std::ranges::range_value_t<option_type>>) &&
std::invocable<validator_type, option_type>
//!\endcond
void add_option(option_type & value,
Expand Down Expand Up @@ -293,7 +293,7 @@ class argument_parser
template <typename option_type, validator validator_type = detail::default_validator<option_type>>
//!\cond
requires (argument_parser_compatible_option<option_type> ||
argument_parser_compatible_option<typename option_type::value_type>) &&
argument_parser_compatible_option<std::ranges::range_value_t<option_type>>) &&
Copy link
Member Author

Choose a reason for hiding this comment

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

this is just a drive-by fix

std::invocable<validator_type, option_type>
//!\endcond
void add_positional_option(option_type & value,
Expand Down
76 changes: 38 additions & 38 deletions include/seqan3/argument_parser/validators.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,17 @@ namespace seqan3
* \brief You can expect these (meta-)functions on all types that implement seqan3::validator.
* \{
*/
/*!\typedef using value_type
/*!\typedef using option_value_type
* \brief The type of value on which the validator is called on.
* \relates seqan3::validator
*
* \details
* \attention This is a concept requirement, not an actual typedef (however types satisfying this concept
* will provide an implementation).
*/
/*!\fn void operator()(value_type const & cmp) const
/*!\fn void operator()(option_value_type const & cmp) const
* \brief Validates the value 'cmp' and throws a seqan3::validation_failed on failure.
* \tparam value_type The type of the value to be validated.
* \tparam option_value_type The type of the value to be validated.
* \param[in,out] cmp The value to be validated.
* \relates seqan3::validator
* \throws seqan3::validation_failed if value 'cmp' does not pass validation.
Expand All @@ -84,9 +84,9 @@ namespace seqan3
template <typename validator_type>
SEQAN3_CONCEPT validator = std::copyable<remove_cvref_t<validator_type>> &&
requires(validator_type validator,
typename std::remove_reference_t<validator_type>::value_type value)
typename std::remove_reference_t<validator_type>::option_value_type value)
{
typename std::remove_reference_t<validator_type>::value_type;
typename std::remove_reference_t<validator_type>::option_value_type;

{ validator(value) } -> void;
{ validator.get_help_page_message() } -> std::string;
Expand All @@ -97,8 +97,6 @@ SEQAN3_CONCEPT validator = std::copyable<remove_cvref_t<validator_type>> &&
* \ingroup argument_parser
* \implements seqan3::validator
*
* \tparam option_value_type Must be a (container of) arithmetic type(s).
*
* \details
*
* On construction, the validator must receive a maximum and a minimum number.
Expand All @@ -111,21 +109,21 @@ class arithmetic_range_validator
{
public:
//!\brief The type of value that this validator invoked upon.
using value_type = double;
using option_value_type = double;

/*!\brief The constructor.
* \param[in] min_ Minimum set for the range to test.
* \param[in] max_ Maximum set for the range to test.
*/
arithmetic_range_validator(value_type const min_, value_type const max_) :
arithmetic_range_validator(option_value_type const min_, option_value_type const max_) :
min{min_}, max{max_}
{}

/*!\brief Tests whether cmp lies inside [`min`, `max`].
* \param cmp The input value to check.
* \throws parser_invalid_argument
*/
void operator()(value_type const & cmp) const
void operator()(option_value_type const & cmp) const
{
if (!((cmp <= max) && (cmp >= min)))
throw parser_invalid_argument(detail::to_string("Value ", cmp, " is not in range [", min, ",", max, "]."));
Expand Down Expand Up @@ -154,15 +152,16 @@ class arithmetic_range_validator

private:
//!\brief Minimum of the range to test.
value_type min{};
option_value_type min{};

//!\brief Maximum of the range to test.
value_type max{};
option_value_type max{};
};

/*!\brief A validator that checks whether a value is inside a list of valid values.
* \ingroup argument_parser
* \implements seqan3::validator
* \tparam option_value_t Must be a (container of) arithmetic type(s).
*
* \details
*
Expand All @@ -176,12 +175,12 @@ class arithmetic_range_validator
*
* \include test/snippet/argument_parser/validators_2.cpp
*/
template <typename option_value_type>
template <typename option_value_t>
class value_list_validator
{
public:
//!\brief Type of values that are tested by validator
using value_type = option_value_type;
using option_value_type = option_value_t;

/*!\name Constructors, destructor and assignment
* \{
Expand All @@ -194,7 +193,7 @@ class value_list_validator
~value_list_validator() = default; //!< Defaulted.

/*!\brief Constructing from a range.
* \tparam range_type The type of range; must model std::ranges::forward_range and value_list_validator::value_type
* \tparam range_type The type of range; must model std::ranges::forward_range and value_list_validator::option_value_type
* must be constructible from the rvalue reference type of the given range.
* \param[in] rng The range of valid values to test.
*/
Expand All @@ -209,7 +208,7 @@ class value_list_validator
}

/*!\brief Constructing from a parameter pack.
* \tparam option_types The type of option values in the parameter pack; The value_list_validator::value_type must
* \tparam option_types The type of option values in the parameter pack; The value_list_validator::option_value_type must
* be constructible from each type in the parameter pack.
* \param[in] opts The parameter pack values.
*/
Expand All @@ -227,7 +226,7 @@ class value_list_validator
* \param cmp The input value to check.
* \throws parser_invalid_argument
*/
void operator()(value_type const & cmp) const
void operator()(option_value_type const & cmp) const
{
if (!(std::find(values.begin(), values.end(), cmp) != values.end()))
throw parser_invalid_argument(detail::to_string("Value ", cmp, " is not one of ", std::views::all(values), "."));
Expand Down Expand Up @@ -256,7 +255,7 @@ class value_list_validator
private:

//!\brief Minimum of the range to test.
std::vector<value_type> values{};
std::vector<option_value_type> values{};
};

/*!\brief Type deduction guides
Expand Down Expand Up @@ -314,7 +313,7 @@ class file_validator_base
public:

//!\brief Type of values that are tested by validator.
using value_type = std::string;
using option_value_type = std::string;

/*!\name Constructors, destructor and assignment
* \{
Expand All @@ -336,7 +335,7 @@ class file_validator_base
*/
virtual void operator()(std::filesystem::path const & path) const = 0;

/*!\brief Tests whether every path in list \p v passes validation. See operator()(value_type const & value)
/*!\brief Tests whether every path in list \p v passes validation. See operator()(option_value_type const & value)
* for further information.
* \tparam range_type The type of range to check; must model std::ranges::forward_range and the value type must
* be convertible to std::filesystem::path.
Expand Down Expand Up @@ -473,7 +472,7 @@ class input_file_validator : public file_validator_base
"Expected either a template type with a static member called valid_formats (a file type) or void.");

// Import from base class.
using typename file_validator_base::value_type;
using typename file_validator_base::option_value_type;

/*!\name Constructors, destructor and assignment
* \{
Expand Down Expand Up @@ -591,7 +590,7 @@ class output_file_validator : public file_validator_base
"Expected either a template type with a static member called valid_formats (a file type) or void.");

// Import from base class.
using typename file_validator_base::value_type;
using typename file_validator_base::option_value_type;

/*!\name Constructors, destructor and assignment
* \{
Expand Down Expand Up @@ -681,7 +680,7 @@ class input_directory_validator : public file_validator_base
{
public:
// Import from base class.
using typename file_validator_base::value_type;
using typename file_validator_base::option_value_type;

/*!\name Constructors, destructor and assignment
* \{
Expand Down Expand Up @@ -753,7 +752,7 @@ class output_directory_validator : public file_validator_base
{
public:
// Imported from base class.
using typename file_validator_base::value_type;
using typename file_validator_base::option_value_type;

/*!\name Constructors, destructor and assignment
* \{
Expand Down Expand Up @@ -838,7 +837,7 @@ class regex_validator
{
public:
//!\brief Type of values that are tested by validator.
using value_type = std::string;
using option_value_type = std::string;

/*!\brief Constructing from a vector.
* \param[in] pattern_ The pattern to match.
Expand All @@ -851,7 +850,7 @@ class regex_validator
* \param[in] cmp The value to validate.
* \throws parser_invalid_argument
*/
void operator()(value_type const & cmp) const
void operator()(option_value_type const & cmp) const
{
std::regex rgx(pattern);
if (!std::regex_match(cmp, rgx))
Expand All @@ -866,7 +865,7 @@ class regex_validator
*/
template <std::ranges::forward_range range_type>
//!\cond
requires std::convertible_to<std::ranges::range_value_t<range_type>, value_type const &>
requires std::convertible_to<std::ranges::range_value_t<range_type>, option_value_type const &>
//!\endcond
void operator()(range_type const & v) const
{
Expand All @@ -890,20 +889,21 @@ namespace detail
/*!\brief Validator that always returns true.
* \ingroup argument_parser
* \implements seqan3::validator
* \tparam option_value_t Must be a (container of) arithmetic type(s).
*
* \details
*
* The default validator is needed to make the validator parameter of
* argument_parser::add_option and argument_parser::add_option optional.
*/
template <typename option_value_type>
template <typename option_value_t>
struct default_validator
{
//!\brief Type of values that are tested by validator
using value_type = option_value_type;
using option_value_type = option_value_t;

//!\brief Value cmp always passes validation because the operator never throws.
void operator()(option_value_type const & /*cmp*/) const noexcept
void operator()(option_value_t const & /*cmp*/) const noexcept
{}

//!\brief Since no validation is happening the help message is empty.
Expand All @@ -919,20 +919,20 @@ struct default_validator
*
*\details
*
* Note that both validators must operate on the same value_type in order to
* Note that both validators must operate on the same option_value_type in order to
* avoid unexpected behaviour and ensure that the seqan3::argument_parser::add_option
* call is well-formed. (add_option(val, ...., validator) requires
* that val is of same type as validator::value_type).
* that val is of same type as validator::option_value_type).
*/
template <validator validator1_type, validator validator2_type>
//!\cond
requires std::same_as<typename validator1_type::value_type, typename validator2_type::value_type>
requires std::same_as<typename validator1_type::option_value_type, typename validator2_type::option_value_type>
//!\endcond
class validator_chain_adaptor
{
public:
//!\brief The underlying type in both validators.
using value_type = typename validator1_type::value_type;
using option_value_type = typename validator1_type::option_value_type;

/*!\name Constructors, destructor and assignment
* \{
Expand Down Expand Up @@ -992,10 +992,10 @@ class validator_chain_adaptor
* \ingroup argument_parser
* \tparam validator1_type The type of the fist validator;
* Must satisfy the seqan3::validator and the
* same value_type as the second validator type.
* same option_value_type as the second validator type.
* \tparam validator2_type The type of the second validator;
* Must satisfy the seqan3::validator and the
* same value_type as the fist validator type.
* same option_value_type as the fist validator type.
* \param[in] vali1 The first validator to chain.
* \param[in] vali2 The second validator to chain.
* \returns A new validator that tests a value for both vali1 and vali2.
Expand All @@ -1017,8 +1017,8 @@ class validator_chain_adaptor
*/
template <validator validator1_type, validator validator2_type>
//!\cond
requires std::same_as<typename std::remove_reference_t<validator1_type>::value_type,
typename std::remove_reference_t<validator2_type>::value_type>
requires std::same_as<typename std::remove_reference_t<validator1_type>::option_value_type,
typename std::remove_reference_t<validator2_type>::option_value_type>
//!\endcond
auto operator|(validator1_type && vali1, validator2_type && vali2)
{
Expand Down