Home

Sprint 2018/11/26 - 2018/12/17 by marcel (@marehr)

quick facts:

• 3 open PRs (~33 comments)
• 15 merged PRs (~161 comments, merged:2018-11-26..2018-12-17)
• altogether ~194 comments (~11 comments per pr)
• 11 reviews
• 2 open reviews

3 major projects:

• modularizing alphabet test cases (opened Oct 29th)
• introduce code coverage test and nice overview (opened Jun 6th)
• (re-)introduce strong warning levels (opened Dec 5th)

modularizing alphabet test cases

each concept models something

=> all types which model the concept should only include a concept ("meta") tests.

How we did this before:

• name concept tests as alphabet_test.cpp, ...
• use TYPED_TEST_CASE
• add all types which model the concept in that test

Cons with this approach:

• each added type increases build time (by a lot!)
• hard to maintain, it's easy to forget to add a new type
• if one type breaks, because of compiler changes, it is a mess to locate the problem
• the name alphabet_test did not imply any model test

How we change it:

• add a new alphabet_test_template.hpp file which contains all tests for a concept/model
• each type includes in its own test this header
• and instantiates it

Example, template for a concept test:

https://github.com/seqan/seqan3/blob/c7aa6a711c0b0d6a8bc8eff4a62034403eeb512c/test/unit/alphabet/alphabet_test_template.hpp#L30-L39

Example, type which includes the concept test:

https://github.com/seqan/seqan3/blob/c7aa6a711c0b0d6a8bc8eff4a62034403eeb512c/test/unit/alphabet/nucleotide/dna4_test.cpp#L8-L14

Pros:

• reduces build time (by enabling parallel builds)
• easy to add a new type
• compiler problems with a type is more locally now
• better name for model tests
• (each header file has now it's own test cases; easier to investigate problems)

Cons:

• the definition of those concept tests is a bit harder

  https://github.com/seqan/seqan3/blob/c7aa6a711c0b0d6a8bc8eff4a62034403eeb512c/test/unit/alphabet/alphabet_test_template.hpp#L292-L294

introduce code coverage test and nice overview

Just look at https://github.com/seqan/seqan3/pull/607#issuecomment-447756161

and then at https://codecov.io/gh/seqan/seqan3

(re-)introduce strong warning levels

We have strong warning levels again.

I fixed mostly

• unsigned != signed comparison warnings.
• stmt1 && stmt2 || stmt3 || stmt4 && stmt5 bracket warnings (chris loves them).
• unused parameters warnings
• narrowing conversions warnings (int -> char)
• uninitialized warnings

IMPORTANT

If you encounter problems with our test cases ISO C++11 requires at least one argument for the "..." in a variadic macro [-Werror].

Please checkout 52f8183e7f3620cf03f321a2624eb0d4f7649f4c in <build-folder>/vendor/googletest/.

New detail::API for_each on parameter packs / type lists / types

https://github.com/seqan/seqan3/blob/c7aa6a711c0b0d6a8bc8eff4a62034403eeb512c/test/snippet/core/algorithm/for_each_value.cpp#L1-L28

https://github.com/seqan/seqan3/blob/c7aa6a711c0b0d6a8bc8eff4a62034403eeb512c/test/snippet/core/algorithm/for_each_type.cpp#L1-L54

https://github.com/seqan/seqan3/blob/c7aa6a711c0b0d6a8bc8eff4a62034403eeb512c/test/snippet/core/algorithm/for_each_type_list.cpp#L1-L57

2 projects not finished yet.

• simd (planned for sprint)
• clang support (not planned, but checked once in a while)

There were some hickups and blockers:

• simd types can't be evaluated at compile time

  • gcc can't set simd[i] = 15 within a constexpr loop
  • clang can't even access simd[i] within constexpr context
  • workarounds by constructing them at compile time didn't auto-vectorize
  • clang does not auto-vectorize at all
  • => decided to make algorithms (i.e., min, max, fill, ...) on simd types non-constexpr

• I wanted to introduce a simd literal:

  problem:

  using uint16x8_t = simd_type_t<uint16_t, 8>;
  /*constexpr*/ uint16x8_t a = 5; // this does NOT work for every compiler

  current solution:

  using uint16x8_t = simd_type_t<uint16_t, 8>;
  /*constexpr*/ uint16x8_t a = fill<uint16x8_t>(5); // this does work for every compiler

  cool solution:

  using uint16x8_t = simd_type_t<uint16_t, 8>;
  /*constexpr*/ uint16x8_t a = 5_simd<uint16x8_t>; // this would be cool, does not work

  cool(er?) solution:

  using uint16x8_t = simd_type_t<uint16_t, 8>;
  /*constexpr*/ uint16x8_t a = 5_simd; // this would be cool, gave me the following error

  error:

  In file included from simd_literal.cpp:1:
  /seqan3/include/seqan3/core/simd/concept.hpp: In function ‘void d()’:
  /seqan3/include/seqan3/core/simd/concept.hpp:87:35: error: invalid use of incomplete type ‘class b’
       requires std::Same<decltype(a - b), simd_t>;
                                   ~~^~~
  

  I could narrow this down to:

  #include <seqan3/core/simd/simd_literal.hpp>
  class a;
  char b(...);
  void b(a *);
  template <int> class c;
  using namespace seqan3;
  void d() {
    detail::simd_literal<4> e;
    c<sizeof(b(e))>;
  }

  not sure how to fix this! Will report bug at gcc, clang works.

clang support

clang does work for a lot of code already. After the alphabet redesign, clang does not like the defaulted constexpr constructors anymore.