Change '{' regex parsing to work similarly to other engines (#2134)

[szledan]

Changed regex parsing to be able to handle opening braces
as other engines do, and also added a compile flag which
can disable this behaviour.

JerryScript-DCO-1.0-Signed-off-by: Szilard Ledan [email protected]

[galpeter]

What if we switch around the logic and do something like this?

    case LIT_CHAR_PLUS:
    case LIT_CHAR_LEFT_BRACE:
    {
#ifndef ENABLE_REGEXP_STRICT_MODE
      const lit_utf8_byte_t *input_curr_p = parser_ctx_p->input_curr_p;

      lit_utf8_decr (&parser_ctx_p->input_curr_p);
      if (!ecma_is_value_empty (re_parse_iterator (parser_ctx_p, out_token_p)))
      {
          parser_ctx_p->input_curr_p = input_curr_p;

          out_token_p->type = RE_TOK_CHAR;
          out_token_p->value = ch;
          ret_value = re_parse_iterator (parser_ctx_p, out_token_p);

          if (!ecma_is_value_empty (ret_value))
          {
            parser_ctx_p->input_curr_p = input_curr_p;
            ret_value = ECMA_VALUE_EMPTY;
          }
          break;
      }
#else /* ENABLE_REGEXP_STRICT_MODE */
      return ecma_raise_syntax_error (ECMA_ERR_MSG ("Invalid RegExp token."));
#endif /* !ENABLE_REGEXP_STRICT_MODE */

so we can remove the 884-886 line additions

[szledan]

I think we can't remove these lines, because this patch handles only the { braces issue and doesn't catch other iterator characters.

[galpeter]

ahhh yeah! I was to eager to simplify.

[zherczeg]

Do we change the current behaviour here? Personally I would prefer FEATURE_REGEXP_COMPATIBILITY_MODE, and keep it OFF by default.

[szledan]

Yes, now the compatibility mode is the default, so the regexp engine accepts this /{}/ regex pattern while the strict mode does not.
Shall I set the strict mode as default?
(Personally I would prefer the extended behaviour as default, like other engines.)

[zherczeg]

I would like to hear others opinion (kind of voting). I usually prefer to keep the current behaviour but I am not that strict.

[LaszloLango]

I don't have strong preferences. I think the average user won't notice the compatibility features only if they dig into the build system. So enable compatibility mode by default sounds reasonable for me.

[robertsipka]

I agree with @szledan and @LaszloLango, the compatibility features should be enabled by default, but I would change the name also. So my vote is to set the FEATURE_REGEXP_COMPATIBILITY_MODE to ON instead of setting the FEATURE_REGEXP_STRICT_MODE to OFF.

[robertsipka]

Sorry, I rethought it, the FEATURE_REGEXP_STRICT_MODE with OFF is fine. The other version may cause problems/different behaviors if somebody want to compile without CMake and forget to define the appropriate macro. LGTM (informally)

[LaszloLango]

LGTM

[zherczeg]

LGTM