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
Use C++17's [[fallthrough]] #4230
Conversation
tests/mpi/tria_copy_triangulation.cc
Outdated
// assert_tria_equal("tria_copy_triangulation", tr, new_tr); | ||
|
||
Assert (tr.n_active_cells() == new_tr.n_active_cells(), ExcInternalError()); | ||
Assert (tr.n_levels() == new_tr.n_levels(), ExcInternalError()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reasons to remove these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just moved up. In cell1 != tr.end(), cell2 != new_tr.end();
the first statement was never used. However, it should be sufficient to only have one of these two if we first check that the number of active cells is the same.
Let me extract this in another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to #4231
include/deal.II/base/table_indices.h
Outdated
Assert (index1 == numbers::invalid_unsigned_int, ExcMessage("more than N index values provided")); | ||
case 2: // fallthrough | ||
DEAL_II_FALLTHROUGH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make the life of IDEs easier, what would you think of putting the semicolon here instead of the definition of the macro? If the macro is defined as empty, then this just yields an empty statement. If it is defined as something nonempty, it results in the same as right now. In either case, anything that doesn't know that it's a macro will think of it as a regular statement and indent it correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine with me. At least astyle
seems to already handle this correctly in either form.
Would you mind splitting this into a separate PR to just add C++17 support? What you have looks right and we can probably get that merged quicker. |
@drwells Sure, I'll split this up if the C++17 support part already looks good to you. |
You'll have to rebase this one now that #4232 is merged. |
Rebased |
Based on #4232.
Currently,
gcc-7
(-Wimplicit-fallthrough
) produces tons of warnings because of not handling these fallthrough/break issues. Although there are also warnings from other libraries during build, this PR silences the warnings appearing when compiling user applications.In the end, we probably need to handle the warnings from other libraries (set a proper
Wimplicit-fallthrough=n
level)