ping

ping

The first parameter of pubsetbuf method is also a pointer, so zero is changed to` nullptr`. Missed this in the original patch.

Detected one more file missing this header. Diff is updated.

I have no commit access to Monorepo. Somebody should commit this revision. Thanks.

ping

I don't understand the request. Please, clarify. Also, it would help if you use the terms from the comments in the suggested test.

In D56498#1512443, @ldionne wrote:

I'm going to commit this for you, however you should consider getting commit access (https://llvm.org/docs/DeveloperPolicy.html) since you've done a couple of patches.

Thanks for suggestion. I'll consider it.

More simplifications accepted and committed in D56498 are implemented here: common function moved to a separate header file. Now this patch can also be accepted and committed.

Yet another personal ping for @EricWF. Let's finish up with this finally and proceed with the project.

In D56498#1507473, @EricWF wrote:

I'll commit this is the morning.

Would somebody commit this, please?

Another personal ping for @EricWF. If you have any questions, please ask. If not, please accept and commit.

Ping for @EricWF: project must go on.

If there are still questions/suggestions, please ask/post.

We are now very close to the end here. Let's finish this up.

In D61192#1480548, @mclow.lists wrote:

If you agree, I can just make the changes myself.

Yes, please.

Suggestions implemented. Common functions moved to a separate header file. This header will also be used to simplify code in patch D56500 after this one is accepted and committed.

Suggested simplification implemented. It also implemented for the associated patch D56498.

Simplification suggested by Marshall Clow implemented.

If you have any further questions, please ask. If not, please accept and commit.

ping

ping

Agree. Patch modified as suggested.

Your change list looks incomplete. Consider using grep:

Is the patch rejected due to being "difficult to follow"?

Patch is rebased as requested.

Oops, seems like a "commit race" happened. :-)

// UNSUPPORTED: libcpp-no-exceptions added. I ran the new test against libc++ in c++98 and c++03 modes once again to make sure it passes. It means that // UNSUPPORTED: c++98, c++03; is not needed for the reason I mentioned in my previous comment.

In D57391#1377598, @mclow.lists wrote:

Other than the missing // UNSUPPORTED bits, this looks good to me.
Do you need me to commit this?

The answer is here. If you don't agree with it, please, suggest simpler patch with the same test coverage.

Wouldn't it be enough (and simpler) to do something like this

ping

ping

Yes, I am. The same is true for all other my patches awaiting review.

ping

For complete consistency with this request, two more tests were patched. Calls to strcmp() replaced with std::strcmp() and <cstring> header added according to the requirement of the standard. The test std/depr/depr.c.headers/string_h.pass.cpp remains untouched since it checks the requirements to <string.h> header.

ping

With respect to this particular patch, I would rather wait for the final verdict on the Clang bug Marshall reported recently. If it is a real bug in compiler, than the patch should be considered as both a portability fix and a bug catcher.

I don't use clang. The log of running tests including poisoned_hash_helper.hpp is attached.

In D55777#1334589, @mclow.lists wrote:

The includes look fine. The static_casts (as always) look ugly. Once you make the changes that I've requested, you can commit this.

The requested changes applied. I can't commit the patch myself: I don't have rights to do this.

Is this patch rejected? BTW, since git repository is used to store tests, the copy of any previous version of the patched tests can be made any time after the patch is applied.

In D55366#1321692, @mclow.lists wrote:

Yes, but that's just true for libc++. Other library maintainers use these tests, too.

It is also true for GNU libstdc++.

Sorry. Fixed.

In libc++ these array iterators declared as follows:

ping

Initializer-list construction replaced with insert() throughout the tests.

In D54838#1308594, @ldionne wrote:

Could you please keep the order-dependent tests in a suitably-named file under test/libcxx/<...> instead of test/std/<...>? Tests under test/libcxx/ are libc++-specific.

I would suggest someone from libc++ test suite developers/maintainers team to do this, if it is necessary for them, before the patch is applied to test/std/<...> tests. I'm not a member of this team, but just yet another external user of test/std/<...> tests. So, I'd like to avoid intervening into another expert team's job by trying to solve their internal tasks having no sufficient expertize to do this.

Hmm, I'm confused by this change request. All the tests I patched are intended to test unordered_multimap container, which was introduced in C++11. So, they won't compile in C++03 mode in any case. Please, clarify the reason for the patch change.

Missing space restored. Type name changed according to the coding standard (sorry, I prefer readable identifiers by default).

In D54806#1305601, @ldionne wrote:

However, I don't this SCARY iterators are actually required by the standard (am I wrong?).

BTW, which tests in the test suite are meant to check for SCARY iterators?

typedef added throughout the test to avoid confusion whether the implemented test actions are meant to check for SCARY iterators.

My intention was to be consistent with the #if TEST_STD_VER >= 11 segment of the test, which does not introduce any typedef in the similar context. Should I change both?

Look also at this revision.

No, I don't.