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

validate.cpp: static_assert is_sorted() for the lookup tables #455

Closed
StephanTLavavej opened this issue Jan 24, 2020 · 8 comments · Fixed by #650
Closed

validate.cpp: static_assert is_sorted() for the lookup tables #455

StephanTLavavej opened this issue Jan 24, 2020 · 8 comments · Fixed by #650
Labels
fixed Something works now, yay! good first issue Good for newcomers infrastructure Related to repository automation

Comments

@StephanTLavavej
Copy link
Member

When VS 2019 16.6 Preview 1 is available, the shipping toolset will support #6 constexpr is_sorted(). At that time, this will become a "good first issue".

  1. We'll need to build the tools with /std:c++latest. This is accomplished by telling CMake to use C++20 instead of C++17:

set(CMAKE_CXX_STANDARD 17)

  1. Then we can remove this comment ("TRANSITION" means "TODO" for us), and change each assert to static_assert:

// TRANSITION, P0202R3, use constexpr is_sorted()
assert(is_sorted(skipped_directories.begin(), skipped_directories.end()));
assert(is_sorted(skipped_extensions.begin(), skipped_extensions.end()));
assert(is_sorted(tabby_filenames.begin(), tabby_filenames.end()));

(Related to #224.)

@BillyONeal
Copy link
Member

There's no reason the tools can't use the live STL headers.

@StephanTLavavej
Copy link
Member Author

Mixing live headers with installed libs would be doom. Doom!

@BillyONeal
Copy link
Member

I mean, it wouldn't be a supported configuration but the only people it might break would be ourselves

@StephanTLavavej
Copy link
Member Author

I'm wary of introducing little bits of weirdness - such things accumulate and make systems difficult to deal with. Waiting is simpler.

@BillyONeal
Copy link
Member

Sure! Wasn't saying "do this thing", was only saying "consider this thing" :)

@StephanTLavavej StephanTLavavej removed the enhancement Something can be improved label Feb 6, 2020
@bhardwajs
Copy link
Contributor

@StephanTLavavej - I can work on this issue. Should I assign this to myself?

@StephanTLavavej
Copy link
Member Author

Thanks! You can just submit a pull request - we don't usually assign issues to devs outside the vclibs team. A comment is sufficient to indicate to other readers that you're working on an issue.

@bhardwajs
Copy link
Contributor

Great. Will submit a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed Something works now, yay! good first issue Good for newcomers infrastructure Related to repository automation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants