Ping.

Avoid using std::distance.

In D75905#1918677, @zoecarver wrote:

It would be nice if we could remove the calls to std::difference and replace them with a count variable so that we only have to iterate once. It may require adding a __libcpp_ erase function, though.

Fix typo.

Avoid using overloaded comma operator.

Rebase to master.

Ping.

The pre-merge build results show this build as failed because of failing clang-format linter tests (https://reviews.llvm.org/harbormaster/build/52549/, https://results.llvm-merge-guard.org/amd64_debian_testing_clang8-4604/summary.html).
However, these "failures" are all misindentations of preprocessor directives in generated files in libcxx/test/std/language.support/support.limits/support.limits.general/.
That's out of scope of this patch, but isn't the real fix modifying .clang-format config file to indent PP directives (or having a local copy of .clang-format in tests)?

Fix __cpp_lib_erase_if value.

Fix tests with custom allocators. Fix assert in map test.

Fix string.

Fixed forgotten return type tests.

I've fixed the expected error mesages and relanded it in commit 5e7017273f40a48453ebb758e54123e31f751806 (https://reviews.llvm.org/rGe93e58c6c40a365bc9275dd8a8242598343cc835).

Reverted.

Ping. :)

Ping. :)

Ping. :)

What's the usual way of proceeding, should I land the patch or wait for Louis' approval?

• Add tests. Update Returns commment according to [time.cal.ymwd.members]/21.

Just as a side-note, @ldionne, I have commit access, so I can land the patches, but do as you prefer (when the patch gets accepted) :).

• Make return type consistent.

Rebase.

• Fix typo in comment.

• Readd noexcept.
• Fix message.

Fix patch diff.

Adressed comments.

• Add synopsis.

• Fix constexpr and return type.
• Fix tests.

I'm still not sure what to do about noexcept for to_array.
libstdc++ adds noexcept: https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/array#L361
MSVC STL does not add noexcept: https://github.com/microsoft/STL/blob/master/stl/inc/array#L429
It seems to be a low-hanging fruit to add noexcept(is_nothrow_copy_constructible_v<_Tp>) / noexcept(is_nothrow_move_constructible_v<_Tp>).
What do you think?

• Fix typos.

Updated with tests from D66103. Answered issues from comments.

• Clean up to_array implementation.
• Add tests from D66103. Clean up.

Oh, I didn't know it, I only searched for to_array in master branch before starting.
How should I proceed? I let you take out of my patch what can be useful?
I see that you didn't touch feature test macros at all for instance. And as you said, you might add some tests I added and you seem not having (e.g. fail tests).

LGTM. Just a small typo in comment. Otherwise, great job!

Minor comments.

LGTM.
Any ideas for improvement, @JonasToth?
I'd rather have it merged and improve on it later if there are ideas on how to do it better.

A few more ideas for enhancements.

LGTM. But I'm not a code owner here and I don't know if you need an acceptance of one of them.
Great job.

Some more minor remarks. I'll give this check a try to see how it behaves on some of my projects.
I agree that a high rate of false positives is possible (and is a bit of spoiler) but I wouldn't reject this IMO useful check because of that.
Anyway, everything looks pretty clean for me.

Just a few minor remarks and a possible workaround for testing CHECK-FIXES: [[nodiscard]].

Fixed diff.

Applied changes as per comments.

Actually, I can make it an option for this check to skip or not constexpr ifs, WDYT?

In D53372#1267728, @lebedev.ri wrote:

I think it would be good to add some more explanation as to *why* that else has to be kept.

Thanks for the changes. LGTM.

Any thoughts on this?

Could you add just one more test for PAS_Middle please? A single line will be just enough.
Otherwise, LGTM.

That's precisely what I've written, but, as I'd said before, such tests pass already without any modification in NamespaceEndCommentsFixer.

This was fixed by https://reviews.llvm.org/rL312904 and other commits.

I confirm what I observed before: when invoking tests in unittests/Format/NamespaceEndCommentsFixerTest.cpp, the const AnnotatedLine *line parameter in getNamespaceToken gets one big line that includes both namespace and { (something like namespace\n{\n..., whereas in tests invoked from unittests/Format/FormatTests.cpp, there is a separate line with namespace\n and another one with {\n.

Ok. Nice patch. You can close https://bugs.llvm.org/show_bug.cgi?id=19986 now.

Minor: use FormatToken::isNot instead of !FormatToken::is.

There is one big missing thing here: PointerAlignment. Actually only PAS_Left is taken into account.
There are 3 possible options:
Left: auto& [a, b] = f();
Middle: auto & [a, b] = f();
Right: auto &[a, b] = f();

In D37904#873860, @krasimir wrote:

Could you please move the test that adds a namespace end comment to unittests/Format/NamespaceEndCommentsFixerTest.cpp?

Superseded by rL312125.

Fix: use do-while loop.

Extract method.

Fix line endings again.

Revert unintended format changes.

Fix line endings.