[libcxx-commits] [PATCH] D62451: Regex backreference [1/3] Fixes backreferences for extended grammar.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 2 inline comments as done.
Mordante added inline comments.


================
Comment at: libcxx/include/regex:3540
                         __first = __parse_awk_escape(++__first, __last);
+                    else
+                    {
----------------
mclow.lists wrote:
> This looks like a copy/pasta of `__first = __parse_BACKREF(__first, __last)` to me.
> How is it different?
It is a copy-paste of the body of `__parse_BACKREF` . The `__parse_BACKREF` is used in the basic regex (RE) part of the code. It searches for an entire backreference ( `\n`  where  `n`  is a single digit). This code is part of the extended regex (ERE) and already has found a `\` so only needs to find the digit.

I can move this block to a seperate function and adjust `__parse_BACKREF`. However I felt this would be a bit awkward due to updating `__first`. We can do it like this:
    if(__validate_backref(*__temp))
      __first = ++__temp;
or this: `__first = __validate_backref(__first, __temp);`
But then the `__validate_backref` has a `__first, __current` instead of  a `__first, __last` set of iterator arguments.

Do you want me to move the common part to a separate function? If yes which of the two APIs do you prefer or do you have an alternative? Then I also need to adjust D62453 to move the `__throw_regex_error` the new function.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62451/new/

https://reviews.llvm.org/D62451