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.

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

Removed several warnings when compiling with GCC 10.2 #8

Closed
wants to merge 2 commits into from

Conversation

joka921
Copy link

@joka921 joka921 commented Apr 25, 2021

  • stxxl::priority_queue uses std::memmove internally, which is only defined behavior for trivially copyable types.
    std::pair and std::tuple are not trivially copyable, even if all the stored types are trivially copyable, so storing
    them in a stxxl::priority_queue is technically undefined behavior. While this should still work as expected in all "sane"
    compilers, it is still against the standard and GCC 10.2 throws a warning. This commit fixes the only occurence of this warning
    in the "core" libraries stxxl and stxxl_tools. Note that there are many more occurences of this issues in the examples
    (e.g. in the skew3 algorithm) which remain unchanged by this commit.

  • stxxl::vector::const_vector_iterator had a user-defined copy-constructor but the automatically generated copy-assignment-operator.
    This throws a warning in GCC 10.2 (Typically, either both can be defaulted, or both have to be manually defined). The const_vector_iterator
    is a trivial type, so defaulting both operations works here and leads to a cleaner interface.

- stxxl::priority_queue uses std::memmove internally, which is only defined behavior for trivially copyable types.
  std::pair and std::tuple are not trivially copyable, even if all the stored types are trivially copyable, so storing
  them in a stxxl::priority_queue is technically undefined behavior. While this should still work as expected in all "sane"
  compilers, it is still against the standard and GCC 10.2 throws a warning. This commit fixes the only occurence of this warning
  in the "core" libraries stxxl and stxxl_tools. Note that there are many more occurences of this issues in the examples
  (e.g. in the skew3 algorithm) which remain unchanged by this commit.

- stxxl::vector::const_vector_iterator had a user-defined copy-constructor but the automatically generated copy-assignment-operator.
  This throws a warning in GCC 10.2 (Typically, either both can be defaulted, or both have to be manually defined). The const_vector_iterator
  is a trivial type, so defaulting both operations works here and leads to a cleaner interface.
@joka921
Copy link
Author

joka921 commented Apr 25, 2021

@bingmann We are currently using this fork of stxxl for the QLever project (@niklas88 suggested this switch quite some time ago). It seems to be newer than the "official" stxxl/stxxl repository, although both of them have slightly diverged. Could you please comment on the current status of stxxl? Is it still being officially maintained, and if yes, by whom?

@joka921 joka921 closed this Oct 6, 2021
@joka921 joka921 deleted the f.removeWarnings branch October 6, 2021 09:16
@bensuperpc
Copy link

bensuperpc commented Nov 8, 2021

@bingmann We are currently using this fork of stxxl for the QLever project (@niklas88 suggested this switch quite some time ago). It seems to be newer than the "official" stxxl/stxxl repository, although both of them have slightly diverged. Could you please comment on the current status of stxxl? Is it still being officially maintained, and if yes, by whom?

I also ask myself the question, is the project still alive ? I would be interested in maintaining it, but if not many people are interested I don't know if it's worth it.

I thought I would do as I did on this project:
stbrumme/crc32#11

  • Add github action for Linux, MacOS, Windows
  • Add github action for cross platform (ARM, RISCV, PowerPC ect...) with dockcross
  • Add Google test and Google benchmark, to avoid regressions with units test
  • Add C++20 support
  • Update CMake to 3.x
    ...

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