Rename concepts to be standard conform #1860

wvdtoorn · 2020-05-29T05:22:40Z

Renaming the stuff that was renamed by the standard or was in the wrong namespace.

std::default_constructible -> std::default_initializable
std::readable -> std::indirectly_readable
std::writable -> std::indirectly_writable
std::back_inserter -> std::cpp20::back_inserter (back_inserter has a new behaviour since cxx20; it is default initializable)
std::ranges::default_sentinel -> std::default_sentinel (we wrongly assume that these are in std::ranges namespace, even though they are in std namespace)
std::ranges::default_sentinel_t -> std::default_sentinel_t (we wrongly assume that these are in std::ranges namespace, even though they are in std namespace)
std::ranges::all_view -> std::views::all_t

codecov · 2020-06-02T07:56:06Z

Codecov Report

Merging #1860 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1860   +/-   ##
=======================================
  Coverage   97.62%   97.62%           
=======================================
  Files         250      250           
  Lines        9507     9507           
=======================================
  Hits         9281     9281           
  Misses        226      226

Impacted Files	Coverage Δ
...e/algorithm/detail/algorithm_executor_blocking.hpp	`100.00% <ø> (ø)`
include/seqan3/core/detail/pack_algorithm.hpp	`100.00% <ø> (ø)`
...clude/seqan3/io/alignment_file/format_sam_base.hpp	`98.58% <ø> (ø)`
include/seqan3/io/alignment_file/input.hpp	`100.00% <ø> (ø)`
include/seqan3/io/alignment_file/output.hpp	`100.00% <ø> (ø)`
include/seqan3/io/sequence_file/format_fasta.hpp	`97.29% <ø> (ø)`
include/seqan3/io/sequence_file/format_fastq.hpp	`98.41% <ø> (ø)`
include/seqan3/io/sequence_file/input.hpp	`100.00% <ø> (ø)`
include/seqan3/io/sequence_file/output.hpp	`100.00% <ø> (ø)`
include/seqan3/io/structure_file/input.hpp	`100.00% <ø> (ø)`
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c84a1ad...fd018ba. Read the comment docs.

simonsasse

Just one question for my understanding and a small commit thing. :)

simonsasse · 2020-06-02T07:49:11Z

include/seqan3/std/concepts

@@ -378,6 +372,17 @@ template < class T >
 SEQAN3_CONCEPT default_constructible = constructible_from<T>;


I do not quite understand the reason why we define two times the same concept and name it differently (line 383). Is it so that the user can still use std::default_constructible ? How will this appear in the Documentation ?

No, you are completely right. This was incorrect and slipped through. Fixed.

Alright but is this correct: It does not matter wether I use the concept default_constructable or default_initializable ? Because line 383 says both concepts are equal right ?
Also the links lead to the same page:
https://en.cppreference.com/w/cpp/concepts/default_constructible
gets redirected to:
https://en.cppreference.com/w/cpp/concepts/default_initializable

As is, correct.
@marehr What is your thought on this situation? Would it maybe be better, instead of adding the default_initializable concept like now, to do:

diff --git a/include/seqan3/std/concepts b/include/seqan3/std/concepts index 112e7307..014a61de 100644 --- a/include/seqan3/std/concepts +++ b/include/seqan3/std/concepts @@ -361,17 +361,6 @@ template < class T, class... Args > SEQAN3_CONCEPT constructible_from = destructible<T> && std::is_constructible_v<T, Args...>; //!\endcond -/*!\interface std::default_constructible <> - * \extends std::constructible_from - * \brief The std::default_constructible concept provides a shorthand for the common case when the question is whether - * a type can be constructed with no arguments. - * \sa https://en.cppreference.com/w/cpp/concepts/default_constructible - */ -//!\cond -template < class T > -SEQAN3_CONCEPT default_constructible = constructible_from<T>; -//!\endcond - /*!\interface std::default_initializable <> * \extends std::constructible_from * \brief The std::default_initializable concept provides a shorthand for the common case when the question is whether @@ -380,7 +369,7 @@ SEQAN3_CONCEPT default_constructible = constructible_from<T>; */ //!\cond template < class T > -SEQAN3_CONCEPT default_initializable = default_constructible<T>; +SEQAN3_CONCEPT default_initializable = constructible_from<T>; //!\endcond /*!\interface std::move_constructible <>

This one should be removed :) and replaced with std::default_initializable

Good catch :)

Oh @wvandertoorn your patch already expressed this :) Awesome :)

simonsasse · 2020-06-02T07:57:32Z

include/seqan3/std/iterator

@@ -244,8 +244,6 @@ inline namespace deprecated
 // // [default.sentinels], default sentinels
 // struct default_sentinel_t;
 // inline constexpr default_sentinel_t default_sentinel{};
-using ::std::default_sentinel_t;


This line should maybe better be removed in the previous commit (7a241a9) , not sure though. 🧐

Definitely, changed it.

simonsasse

Looks good ! 👌

rrahn

lgtm
We should mention though in the changelog that the following changed in accordance with the standard: default_constructible -> default_initializable, readable -> indirectly_readable, writable -> indirectly_writable

rrahn · 2020-06-02T19:05:56Z

Some more missing std::ranges::default_sentinel references:
/home/travis/build/seqan/seqan3/include/seqan3/range/views/minimiser.hpp:141:29: error: ‘default_sentinel’ is not a member of ‘std::ranges’

…nel_t

wvdtoorn · 2020-06-03T10:50:03Z

Some more missing std::ranges::default_sentinel references:
/home/travis/build/seqan/seqan3/include/seqan3/range/views/minimiser.hpp:141:29: error: ‘default_sentinel’ is not a member of ‘std::ranges’

Wow, that was some confusing rebase fuck-up, sorry.. Fixed and changelog entry added.

rrahn

Just one more little change 👍

CHANGELOG.md

rrahn · 2020-06-04T20:31:03Z

Wow, that was some confusing rebase fuck-up, sorry.. Fixed and changelog entry added.

Well, that is what fun looks like 😆

rrahn

Lgtm, many thanks.

wvdtoorn requested review from a team and simonsasse and removed request for a team May 29, 2020 05:23

wvdtoorn force-pushed the rename_deprecated_concepts branch 2 times, most recently from 907a290 to 3c773c3 Compare June 2, 2020 07:08

simonsasse reviewed Jun 2, 2020

View reviewed changes

wvdtoorn force-pushed the rename_deprecated_concepts branch from 3c773c3 to ffd6eb3 Compare June 2, 2020 09:04

wvdtoorn requested a review from simonsasse June 2, 2020 09:13

wvdtoorn force-pushed the rename_deprecated_concepts branch from ffd6eb3 to 66e3283 Compare June 2, 2020 14:48

simonsasse approved these changes Jun 2, 2020

View reviewed changes

wvdtoorn requested review from a team and rrahn and removed request for a team June 2, 2020 15:19

rrahn approved these changes Jun 2, 2020

View reviewed changes

wvdtoorn added 7 commits June 3, 2020 12:23

[FIX] Replace std::ranges::all_view with std::views::all_t

391c4d9

[FIX] Replace std::readable with std::indirectly_readable

fa9951d

[FIX] Replace std::writable with std::indirectly_writable

208dd9e

[FIX] Replace std::default_constructible with std::default_initializable

2cbddc5

[FIX] Replace std::ranges::default_sentinel_t with std::default_senti…

4b2782c

…nel_t

[FIX] Replace std::ranges::default_sentinel with std::default_sentinel

b89c7bd

[FIX] Replace std::ranges::back_inserter with std::cpp20::back_inserter

0d76e6a

wvdtoorn force-pushed the rename_deprecated_concepts branch from 66e3283 to 3dcca4a Compare June 3, 2020 10:49

wvdtoorn requested a review from rrahn June 4, 2020 08:06

rrahn reviewed Jun 4, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

[DOC] Changelog entry for renamed std concepts

fd018ba

wvdtoorn force-pushed the rename_deprecated_concepts branch from 02a5d73 to fd018ba Compare June 5, 2020 06:14

wvdtoorn requested a review from rrahn June 5, 2020 06:16

rrahn approved these changes Jun 5, 2020

View reviewed changes

rrahn merged commit cf2505c into seqan:master Jun 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename concepts to be standard conform #1860

Rename concepts to be standard conform #1860

wvdtoorn commented May 29, 2020 •

edited by marehr

codecov bot commented Jun 2, 2020 •

edited

simonsasse left a comment

simonsasse Jun 2, 2020

wvdtoorn Jun 2, 2020

simonsasse Jun 2, 2020 •

edited

wvdtoorn Jun 2, 2020 •

edited

marehr Jun 2, 2020 •

edited

marehr Jun 2, 2020

simonsasse Jun 2, 2020 •

edited

wvdtoorn Jun 2, 2020

simonsasse left a comment

rrahn left a comment

rrahn commented Jun 2, 2020

wvdtoorn commented Jun 3, 2020

rrahn left a comment

rrahn commented Jun 4, 2020

rrahn left a comment

		@@ -378,6 +372,17 @@ template < class T >
		SEQAN3_CONCEPT default_constructible = constructible_from<T>;

Rename concepts to be standard conform #1860

Rename concepts to be standard conform #1860

Conversation

wvdtoorn commented May 29, 2020 • edited by marehr

codecov bot commented Jun 2, 2020 • edited

Codecov Report

simonsasse left a comment

Choose a reason for hiding this comment

simonsasse Jun 2, 2020

Choose a reason for hiding this comment

wvdtoorn Jun 2, 2020

Choose a reason for hiding this comment

simonsasse Jun 2, 2020 • edited

Choose a reason for hiding this comment

wvdtoorn Jun 2, 2020 • edited

Choose a reason for hiding this comment

marehr Jun 2, 2020 • edited

Choose a reason for hiding this comment

marehr Jun 2, 2020

Choose a reason for hiding this comment

simonsasse Jun 2, 2020 • edited

Choose a reason for hiding this comment

wvdtoorn Jun 2, 2020

Choose a reason for hiding this comment

simonsasse left a comment

Choose a reason for hiding this comment

rrahn left a comment

Choose a reason for hiding this comment

rrahn commented Jun 2, 2020

wvdtoorn commented Jun 3, 2020

rrahn left a comment

Choose a reason for hiding this comment

rrahn commented Jun 4, 2020

rrahn left a comment

Choose a reason for hiding this comment

wvdtoorn commented May 29, 2020 •

edited by marehr

codecov bot commented Jun 2, 2020 •

edited

simonsasse Jun 2, 2020 •

edited

wvdtoorn Jun 2, 2020 •

edited

marehr Jun 2, 2020 •

edited

simonsasse Jun 2, 2020 •

edited