This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Portability fix: regex algorithms exceptions made optional.
ClosedPublic

Authored by amakc11 on Jan 29 2019, 9:18 AM.

Details

Summary

The section r.alg of the standard specifies three regular expressions algorithms: regex_­match, regex_­search and regex_­replace. The preamble section specifies the requirement to optional exception thrown by these algorithms: "The algorithms described in this subclause may throw an exception of type regex_­error". However, the exponential tests for regex_­match and regex_­search consider these exceptions to be mandatory, which makes these tests fail under some conformant implementations. This patch makes the mentioned tests consider these exceptions optional as required by the standard. Additionally, the patch introduces the missing exponential test for regex_­replace algorithm to improve test coverage.

Diff Detail

Repository
rCXX libc++

Event Timeline

amakc11 created this revision.Jan 29 2019, 9:18 AM

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

test/std/re/re.alg/re.alg.replace/exponential.pass.cpp
10

Needs a // UNSUPPORTED: libcpp-no-exceptions

The other "exponential" tests also have a // UNSUPPORTED: c++98, c++03; but I'm not sure why. Please add it here, and I'll investigate.

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

Yes. I don't have commit permissions.

The other "exponential" tests also have a // UNSUPPORTED: c++98, c++03; but I'm not sure why.

These other "exponential" tests use std::regex_constants::syntax_option_type introduced in C++11. The new test I added does not use this type and associated constants, so it does not require this UNSUPPORTED directive. Besides, I made this new test consistent with all other tests in the same directory, which do not have UNSUPPORTED: c++98, c++03 for the same reason. However, if you disagree, I will add it along with UNSUPPORTED: libcpp-no-exceptions, but I'd like to now the reason eventually.

amakc11 updated this revision to Diff 184496.Jan 31 2019, 6:59 AM

// 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.

amakc11 marked an inline comment as done.Jan 31 2019, 6:59 AM
mclow.lists accepted this revision.Jan 31 2019, 10:54 AM

Committed as revision 352781.

This revision is now accepted and ready to land.Jan 31 2019, 10:54 AM
mclow.lists closed this revision.Jan 31 2019, 10:55 AM