10

I was reading the documentation of std::sub_match<BidirectionalIterator> and saw that it publicly inherits from std::pair<BidirectionalIterator, BidirectionalIterator>. Since a sub_match is simply a pair of iterators into a sequence of characters, with some additional functions, I can understand that it is implemented with a pair, but why use public inheritance?

The problem with inheriting publicly from std::pair<T,U> is the same as inheriting publicly from most other standard classes: they are not meant to be manipulated polymorphically (notably they do not define a virtual destructor). Other members will also fail to work properly, namely the assignment operator and the swap member function (they will not copy the matched member of sub_match).

Why did Boost developers and then the committee decided to implement sub_match by inheriting publicly from pair instead of using composition (or private inheritance with using declarations if they wanted to keep member access through first and second)?

Luc Touraille
  • 79,925
  • 15
  • 92
  • 137

5 Answers5

5

It's an interesting question. Presumably, they considered it safe because no one would ever dynamically allocate one anyway. About the only way you're going to get sub_match objects is as a return value from some of the functions of basic_regex, or as copies of other sub_match, and all of these will be either temporaries or local variables.

Note that it's not safe to keep sub_match objects around anyway, since they contain iterators whose lifetime... doesn't seem to be specified in the standard. Until the match_results object is reused? Until the string operand to the function which filled in the match_results object is destructed? Or?

I'd still have avoided the public inheritence. But in this case, it's not as dangerous as it looks, because there's really no reason you'd ever want to dynamically allocate a sub_match.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • I agree on the dynamic allocation that should probably never happen. However, issues could still appear with `=` and `swap`. I was thinking about Boost.Range for instance, but it does not require ranges to be Assignable or Swappable. However, it is interesting to note that Boost.Range algorithms do not accept `sub_match`es as arguments, but do so if they are manipulated through a reference to a `pair` (trait classes issues). – Luc Touraille Oct 27 '11 at 15:49
3

Here's what regex's author has to say about it: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2003/n1429.htm#matches_discussion

Nothing very specific to your question, I'm afraid.

I would guess that this decision was a trade off between reinventing the wheel and introducing a little risk of misuse. Note that in general there is no need to construct a sub_match, they are returned from regex function. Moreover pairs of iterators are a very practical way of implementing ranges.

Nicola Musatti
  • 17,834
  • 2
  • 46
  • 55
3

Because C++ has no way to inherit an interface without public inheritance. You can inherit an implementation with private inheritance, but then everything is private. If you want the same interface as a std::pair, you have to be a std::pair.

Also, consider this. This is obviously undefined behavior:

std::sub_match<BidirectionalIterator> theMatch = ...;
std::pair<BidirectionalIterator> *pMatch = &theMatch;
delete pMatch;

But so is this:

std::sub_match<BidirectionalIterator> theMatch = ...;
std::pair<BidirectionalIterator> *pMatch = &theMatch.pair;
delete pMatch;

Why is the first one so much more of a concern than the second?

sub_match and pair are light-weight objects (depending on their contents of course). They are meant to be copied or passed by reference, all of which is 100% safe. There is little if any reason to allocate them on the heap and use them through pointers. So while I understand your concern, I think it's unlikely to happen in any real code.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • Indeed, your second example is (almost) as flawed as the first one, but this is because you considered that the `pair` member would be public: why would you assume such a bad encapsulation would be used? And while I do agree that the deletion would probably never be an issue, I disagree that passing by reference is 100% safe, because of the other non-virtual member functions. – Luc Touraille Oct 28 '11 at 07:54
  • You *can* inherit (parts) of a public interface using private inheritance:`class sub_match : pair { using pair.first; using pair.second; bool matched; }` – JohannesD Oct 28 '11 at 08:04
  • If you want the same interface as a `std::pair` without being one, you can use private inheritance along with using declarations and/or delegation. Of course, this is much more work for the developer of the subclass. – Luc Touraille Oct 28 '11 at 08:04
0

Because they didn't need virtual destructor? ;-)

Michael Krelin - hacker
  • 138,757
  • 24
  • 193
  • 173
  • Destructors are not the only members that will fail to work properly: assignment operator and swap will both copy only the `pair` members and not those contained by `sub_match` (the `matched` boolean notably). – Luc Touraille Oct 27 '11 at 15:20
  • The authors obviously felt that they didn't need a virtual destructor, or they would have provided one. But why, since in most cases, public inheritance implies the need of a virtual destructor in the base class. – James Kanze Oct 27 '11 at 15:23
  • @LucTouraille Good point. Quite clearly, there is no intent that `sub_match` isA `pair`. – James Kanze Oct 27 '11 at 15:24
  • @JamesKanze: You are right, the "is-a" relationship does not apply to these two classes, the Liskov substitution principle is not respected. – Luc Touraille Oct 27 '11 at 15:52
  • @Luc Touraille: While it's more than reasonable to expect destruction to work correctly for polymorphic objects, I'd say that expecting either assignment or swap to do anything sensible in a polymorphic context is asking for trouble. – Nicola Musatti Oct 27 '11 at 15:55
  • @NicolaMusatti: That is a fair point, which in fact corroborate the fact that inheritance is probably a bad idea here: `pair` and `sub_match` are both types with value semantics, which are meant to be copied around, assigned, etc. As such, I don't think having them in a class hierarchy is such a good design, as it makes the user be careful about slicing issues and the like. – Luc Touraille Oct 27 '11 at 16:20
  • Well, the thing is, that sub_match *is* a pair and nothing else, so it needs no extra functionality. By inheriting it from pair you have a benefit of using it where pair is accepted. – Michael Krelin - hacker Oct 27 '11 at 18:28
  • @MichaelKrelin-hacker: That's the point: it *is* more than a pair. It notably has a `matched` boolean member. And it does not work everywhere a `pair` is accepted (cf. Boost.Range). – Luc Touraille Oct 28 '11 at 07:46
  • Yes, you're right, it's more than a pair. Not sure why would pair not work where pair is accepted, but nevermind. – Michael Krelin - hacker Oct 28 '11 at 08:09
  • @Michael: in the case of Boost.Range, it must be an a problem of template specialization, which does not work very well with inheritance hierarchy. I must admit that this is not very relevant to the subject, since this is more a design flaw of templates. – Luc Touraille Oct 28 '11 at 10:00
0

If std::sub_match<BidirectionalIterator> has no state of its own, then it is fine for it to inherit from std::pair. Don't do it at home though.

Alexandre C.
  • 55,948
  • 11
  • 128
  • 197