Prevent infinite loops for recursive containers like boost::filesystem::path

[Dani-Hub]

This pull request introduces an additional detection for recursively defined containers (such as boost::filesystem::path or std::filesystem::path) and prevents the possibility of an infinite loop within the output handler for containers.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
• In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

[Dani-Hub]

I signed the CLA.

[googlebot]

CLAs look good, thanks!

[gennadiycivil]

Please add tests so we have evidence that this in fact works as intended

[Dani-Hub]

Can you give me a hint about the test guidelines in googletest? First: Where should these be added? Is it expected to add tests to both googletest and googlemock? Thanks.

[Dani-Hub]

I looked into the existing googletest test directory and found that gtest-printers_test.cc is already the right test for this. I will add a corresponding test case to this pre-existing test file.

[Dani-Hub]

I signed the CLA.

[Dani-Hub]

Is there anything that I need to do to realize that the CI build will run again?

[Dani-Hub]

Could someone please review the suggested patch? Thank you!

[gennadiycivil]

Note that appVeyor build failed , could you please take a look what happened

[Dani-Hub]

Yep, I'll fix it this evening.

[Dani-Hub]

Note that the fixed had been applied and the previous build succeeded. I decided to update the branch yesterday (This IC is still in progress), but that seems to be an endless story, so I don't intend to apply further changes and instead await the review approval.

[gennadiycivil]

Thank you for the updates. I fully support small PRs that are much easier to handle. For this PR, could you please add full test information as well as the test output before/after to demonstrate what we are fixing and how works now. Thank you

[Dani-Hub]

I'm not sure that I understand your request, but the set of changes are intended to solve the problem described in issue #521. Does this clarify your question?

[gennadiycivil]

Yes, 521 has information needed.
We just need testing evidence, test before this PR and demonstrate the problem in action ... and after that test again "after* this PR to demonstrate that solution works

[Dani-Hub]

The real-world test will require someone to validate that boost::filesystem::path and std::filesystem::path will cause normal output by the gtest printers, but I cannot make gtest itself dependent on these concrete types, therefore these types are not used in the test. The patch will work for every "recursive" container and that is validated by the added test in gtest-printers_test.cc by means of the most minimal recursive container example denoted as PathLike.

[gennadiycivil]

How about using Alexhuszagh's Jun 19 suggestion (<experimental/filesytem>) ?
We do need a real world test. The best would be adding a new unit test, with appropriate ifdefs to protect against old compilers

[Dani-Hub]

No, <experimental/filesystem> is not a required header, so we cannot rely on the assumption that it is available. It is not part of the C++ standard. I don't understand the difference between the test type and a "real-world" type. I'm not willing to re-implement std::filesystem within a test just as a proof for another possible type in the world that behaves correctly similar to PathLike. What is wrong with PathLike alone?

[SimonEbner]

PR fixed the issue nlohmann/json#709

[gennadiycivil]

Thank you for detailed explanation. lgtm