This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Add test to check min/max requirement to regular expression.
ClosedPublic

Authored by amakc11 on Jun 3 2019, 9:46 AM.

Details

Summary

The original test suite includes test std/re/re.grammar/excessive_brace_count.cpp checking the requirement that any brace count number in regular expression a{n} should be within numeric limits. However, a conforming implementation might fail this test by throwing another exception (for example, error_space) still correctly preventing an application from creation of such an instance of std::regex. This patch adds a test for a{n,m} regular expression forcing a conforming implementation to check the requirement m >= n. Such a check requires converting m and n to integers and thus forces implementation to check numeric limits for both numbers. As a side effect, this patch improves test coverage of regular expressions processing functionality.

Diff Detail

Repository
rL LLVM

Event Timeline

amakc11 created this revision.Jun 3 2019, 9:46 AM
ldionne accepted this revision.Jun 3 2019, 5:23 PM

This seems reasonable to me. We seem to already be passing this test.

This revision is now accepted and ready to land.Jun 3 2019, 5:23 PM
ldionne requested changes to this revision.Jun 3 2019, 5:28 PM

I think separate tests for a >= b and a <= max would be better, though.

This revision now requires changes to proceed.Jun 3 2019, 5:28 PM

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

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

I want to test that n <= m and that n <= LIMIT separately:

// the "n" and "m" in `a{n,m}` should be within the numeric limits.
// requirement "m >= n" should be checked.

#include <regex>
#include <cassert>
#include "test_macros.h"

int main(int, char**) {
  // test that `n <= m`
  for (std::regex_constants::syntax_option_type op :
       {std::regex::basic}) {
    try {
      TEST_IGNORE_NODISCARD std::regex("a\\{3,2\\}", op);
      assert(false);
    } catch (const std::regex_error &e) {
      assert(e.code() == std::regex_constants::error_badbrace);
      LIBCPP_ASSERT(e.code() == std::regex_constants::error_badbrace);
    }
  }
  for (std::regex_constants::syntax_option_type op :
       {std::regex::ECMAScript, std::regex::extended, std::regex::egrep,
        std::regex::awk}) {
    try {
      TEST_IGNORE_NODISCARD std::regex("a{3,2}", op);
      assert(false);
    } catch (const std::regex_error &e) {
      assert(e.code() == std::regex_constants::error_badbrace);
      LIBCPP_ASSERT(e.code() == std::regex_constants::error_badbrace);
    }
  }

  // test that both bounds are within the limit
  for (std::regex_constants::syntax_option_type op :
       {std::regex::basic}) {
    try {
      TEST_IGNORE_NODISCARD std::regex("a\\{100000000000000000000,10000000000000000002\\}", op);
      assert(false);
    } catch (const std::regex_error &e) {
      assert(e.code() == std::regex_constants::error_badbrace);
      LIBCPP_ASSERT(e.code() == std::regex_constants::error_badbrace);
    }
  }
  for (std::regex_constants::syntax_option_type op :
       {std::regex::ECMAScript, std::regex::extended, std::regex::egrep,
        std::regex::awk}) {
    try {
      TEST_IGNORE_NODISCARD std::regex("a{100000000000000000000,10000000000000000002}", op);
      assert(false);
    } catch (const std::regex_error &e) {
      assert(e.code() == std::regex_constants::error_badbrace);
      LIBCPP_ASSERT(e.code() == std::regex_constants::error_badbrace);
    }
  }

  return 0;
}
ldionne accepted this revision.Jun 4 2019, 9:43 AM

I'll fix it when applying the patch.

This revision is now accepted and ready to land.Jun 4 2019, 9:43 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2019, 9:46 AM