C++ Logo

std-proposals

Advanced search

Re: [std-proposals] Public inheritance with banned conversion to base

From: Breno Guimarães <brenorg_at_[hidden]>
Date: Fri, 24 Jan 2025 19:20:36 -0300
I also inherit from some containers to allow them to be forward declared
and reduce the amount of template instantiation. I don't add any new
functions or members so everything works well.
I don't do it for all types but just some vocabulary types.

Not sure if anyone mentioned that usecase yet (or if there is a categorily
better option).

Breno G

Em sex., 24 de jan. de 2025, 17:37, Arthur O'Dwyer via Std-Proposals <
std-proposals_at_[hidden]> escreveu:

> On Thu, Jan 23, 2025 at 6:46 AM Frederick Virchanza Gotham via
> Std-Proposals <std-proposals_at_[hidden]> wrote:
>
>> On Thursday, January 23, 2025, Filip wrote:
>>
>>> Maybe a stupid question but why wouldn’t the STL containers be marked
>>> with final if it is not encouraged to inherit from them?
>>>
>>
>> This is a good question, and if Arthur is right in what he's saying, then
>> Arthur should be authoring a defect report to mark those containers 'final'.
>>
>
> I said (1) it is never *a good idea* to inherit from an STL type; (2) you *should
> not* inherit from STL types. (Except for the ones that are intended for
> use as base classes, e.g. `true_type`, `enable_shared_from_this<T>`,
> `ranges::view_base`, `error_category`, `ostream`, `exception`... The point
> is that the ones that *aren't* intended for use as base classes, um, are
> intended for use *not* as base classes.)
> I didn't say that it's *physically impossible* to inherit from an STL
> type. In fact it is possible. As Giuseppe pointed out, LWG 2514
> <https://cplusplus.github.io/LWG/issue2514> even codified that wording
> explicitly: the Standard *does permit* inheriting from STL types. It's
> just that if you do it, you're going to have a bad time. And at this point
> there may be code in the wild that has in fact opted into that bad time, so
> a vendor who (non-conformingly) placed `final` on types such as
> `std::vector` would cause that code to break. "Prematurely," as it were. We
> can extracurricularly encourage vendors to add `final` and become
> non-conforming, but I don't think we can argue that the *paper standard*
> should require (or even allow) vendors to add `final`, because it would
> break real code — even though that code is *bad* code, it is also
> currently *working* code.
>
> I have written up this and several similar rules ("you *can physically*
> do this, but you *should not* do it, on pain of having your code broken
> by a language upgrade") under the #sd-8-adjacent
> <https://quuxplusone.github.io/blog/tags/#sd-8-adjacent> tag on my blog.
>
> Below, some data points on `final`. Taking all the examples together, I
> think the conclusion here is that there's no obvious path forward for WG21
> to even *deprecate* inheritance from STL types anytime soon. All we can
> do is teach people that they *should* not do it, and hope that they'll
> listen even without being forced.
>
> (1) We absolutely cannot mark `pair` as `final`, because `std::sub_match`
> is specified to inherit from `pair` <https://eel.is/c++draft/re.submatch>.
>
> (2) LWG has added plenty of heroics over the years to make `std::visit`
> support, not just `variant`, but also program-defined types that inherit
> from `variant`. Personally, I would strongly recommend to working
> programmers that *inheriting from `variant` is just as bad an idea as
> inheriting from `vector`*: you can physically do it, but you will have a
> bad time. (Maybe not today, maybe not tomorrow...) However, LWG's heroics
> seem to indicate that they think people out there *do* write classes
> inheriting from `variant`. (Where? I'd like to know!) If we were going to
> mark `variant` as `final`, then for consistency we should also unwind those
> heroics because they would no longer be necessary.
>
> (3) LWG has added heroics over the years to make `std::get` support, not
> just `tuple`, but also program-defined types that inherit from `tuple`.
> Personally, I would strongly recommend to working programmers that *inheriting
> from `tuple` is just as bad an idea as inheriting from `vector`*: you can
> physically do it, but you will have a bad time. (Maybe not today, maybe not
> tomorrow...) However, LWG's heroics seem to indicate that they think people
> out there *do* write classes inheriting from `tuple`. (Where? I'd like to
> know!) If we were going to mark `tuple` as `final`, then for consistency
> we should also unwind those heroics because they would no longer be
> necessary.
>
> (4) `stack`, `queue`, and `priority_queue` have protected members, which
> indicates that LWG at some point intended people to have a reason to
> inherit from them. I've done so in toy code, but never in real life.
>
> (5) `insert_iterator` and `reverse_iterator` have protected members, which
> indicates that LWG at some point intended people to have a reason to
> inherit from them. I claim that anyone inheriting from `reverse_iterator`
> in C++20 will have a bad time — various Ranges things special-case
> `reverse_iterator` and I don't think they have any coherent theory of what
> to do when they see a class *derived from* `reverse_iterator`!
>
> (6) libc++ has several unit-tests which test that e.g. `bitset` does not
> have an `iterator` member typedef, `list` does not have a `base` member
> typedef, and so on. (A recent example.
> <https://github.com/llvm/llvm-project/commit/397707f7188b6df52de1cff85e08e64e3ee5acc3>)
> These unit tests work by e.g. doing multiple inheritance from `bitset` and
> a second base class, and then checking that a name lookup for `iterator` in
> the derived class isn't ambiguous. When `bitset` is marked `final`, this
> tactic no longer works; libc++ would have to invent a new way of testing
> for the absence of a member typedef.
>
> (7) ASIO uses that same technique to test for the *existence* of member
> functions in "is_buffer_sequence.hpp"
> <https://github.com/chriskohlhoff/asio/blob/cfe14d4/asio/include/asio/detail/is_buffer_sequence.hpp#L32-L51>!
> This means that ASIO itself depends on the ability to inherit from
> `std::vector`.
>
> (8) libc++'s unit test "libcxx/iterators/contiguous_iterators.pass.cpp"
> tests that a program-defined type can inherit from `deque::iterator` and
> then override the iterator_category to contiguous_iterator_tag; libc++ will
> then consider it to be a contiguous iterator. But satisfying a concept
> without modeling it is UB, so arguably this is a silly test. I certainly
> think nothing of value would be lost if this test were rewritten to avoid
> inheritance.
>
> (9) libc++ already, non-conformingly, marks their `join_view::iterator` as
> `final`. I don't actually understand what the problem was that they're
> fixing by doing that, but see D142811
> <https://reviews.llvm.org/D142811#inline-1383022> for what little
> public discussion exists.
>
> (10) I modified my local libc++ to add `final` to every container and
> container iterator (including libc++'s __wrap_iter and __bounded_iter), as
> well as bitset, function, move_only_function, tuple, optional, expected,
> *_insert_iterator, move_iterator, and reverse_iterator. All my employer's
> code still builds, as does Boost (AFAICT), as does libc++'s test suite
> modulo the tests described in (2,3,4,5,6,8) above.
>
> (11) But LLVM/Clang itself uses:
> - inheritance from `optional` in MaybeAlign
> <https://github.com/llvm/llvm-project/blob/6ff8091/llvm/include/llvm/Support/Alignment.h#L117> and
> FailureOr
> <https://github.com/llvm/llvm-project/blob/6ff8091/llvm/include/llvm/Support/LogicalResult.h#L76>
> - inheritance from `vector` in GUIDProbeFunctionMap
> <https://github.com/llvm/llvm-project/blob/6ff8091/llvm/include/llvm/MC/MCPseudoProbe.h#L104> and
> ArrowMap
> <https://github.com/llvm/llvm-project/blob/6ff8091/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp#L162>
> - inheritance from `set` in SUSet
> <https://github.com/llvm/llvm-project/blob/6ff8091/llvm/lib/Target/SystemZ/SystemZMachineScheduler.h#L87>
> - inheritance from `list` in PathPieces
> <https://github.com/llvm/llvm-project/blob/6ff8091/clang/include/clang/Analysis/PathDiagnostic.h#L494>
> - inheritance from `pair` in DenseMapPair
> <https://github.com/llvm/llvm-project/blob/6ff8091/llvm/include/llvm/ADT/DenseMap.h#L42> and
> EnvironmentEntry
> <https://github.com/llvm/llvm-project/blob/6ff8091/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h#L35-L36>
> - inheritance from `monostate` (wow!) in WebAssemblyAsmTypeCheck
> <https://github.com/llvm/llvm-project/blob/6ff8091/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h#L32-L34>
> and probably several more.
>
> (12) GCC's codebase seems pretty clean; at a first glance I see only
> - inheritance from `bitset` in the regression tests for #68991,
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68991> whose reproducer
> came from LLVM's codebase (pre-2019 LLVM used to inherit from `bitset`
> <https://github.com/llvm/llvm-project/commit/16b322914a3fb153d0f9828db539eba6172b012d#diff-33071aba3139f7b684f123ee9464b5738fd9ec7059b2aed0e9667c8fcbc7c46cL38>
> but doesn't anymore)
> - inheritance from `set` in gofrontend/gogo.h
> <https://github.com/gcc-mirror/gcc/blob/c20328e/gcc/go/gofrontend/gogo.h#L242>
>
> –Arthur
> --
> Std-Proposals mailing list
> Std-Proposals_at_[hidden]
> https://lists.isocpp.org/mailman/listinfo.cgi/std-proposals
>

Received on 2025-01-24 22:20:49