[libcxx-commits] [PATCH] D63059: Implements multiline regex support
Louis Dionne via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Mar 20 08:38:30 PDT 2020
ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.
Comment at: libcxx/include/regex:2116
+__r_anchor_multiline<_CharT>::__exec(__state& __s) const
To avoid code duplication, we could create a helper function that takes a `bool __multiline` parameter. Then, `__r_anchor_multiline<_CharT>::__exec` would call that helper function with `__multiline` as an argument, and `__r_anchor<_CharT>::__exec` would call the helper function with `false`.
Comment at: libcxx/include/regex:4840
- __end_->first() = new __l_anchor<_CharT>(__end_->first());
+ __end_->first() = new __l_anchor_multiline<_CharT>(__use_multiline(), __end_->first());
__end_ = static_cast<__owns_one_state<_CharT>*>(__end_->first());
Do we ever create a `__l_anchor` anymore? If not, then we could in theory delete that code. Actually, now that I think of it, I believe we could simply add the functionality to `__l_anchor` itself, but rename the class to something else. The idea is:
- No new code needs to use `__l_anchor`, it can all start using the new class
- Existing code using `__l_anchor` already has a vtable for `__l_anchor`, and that contains the old implementation of `execute` they need to keep using to avoid and ABI break.
I believe the ABI break only happens if we change `__l_anchor::execute` without changing the name of the class, since that's when two libraries compiled at different times (and hence with different versions of `__l_anchor`) can disagree on the layout of `__l_anchor`. If we change the name of the class, I think it should all work out.
CHANGES SINCE LAST ACTION
More information about the libcxx-commits