Re: [lmi] [PATCH] Remove unnecessary definitions of pure virtual functions

[Greg Chicares]

On 2017-03-15 00:46, Vadim Zeitlin wrote:
> 
>  One last patch to fix MSVC build for tonight: this one deals with the same
> noreturn-used-in-the-definition-but-not-in-the-declaration problem as the
> previous one, but in a quite different way: it simply removes the noreturn
> attribute because it also completely removes the function definition which
> seems to be completely unnecessary.
> 
>  This seemed difficult to believe because I thought there must have been
> some reason for having it, but things seem to compile (using MSVC and gcc)
> and work (both interactively and the unit tests) just fine without it and
> the commit a77e4974b92148155158c23687f58df3424a148e which added these
> definitions didn't really explain why did it do it and, contrary to its
> message, this wasn't a simple refactoring as the code it added, and which
> is removed by the attached patch, hadn't been present before it. So it

The addition of dead code doesn't count as a refactoring? Well, I guess
that's one of those "theological" questions, like the distinction between
heresy by thought and heresy by deed.

> seems like it either was a mistake or it was needed 7 years ago, but
> something has changed since then and it's not needed any longer. Or I'm
> missing something, of course, but this is always an implicit possibility.
> 
>  Could you please let me know what am I missing, if anything, or apply this
> patch otherwise?

I believe I'll just commit it (after testing of course).

Clearly these are implementations of pure virtual functions. AFAIK, the only
reason for doing that is to provide a default implementation that might be
appropriate for some derived class to call. But these don't actually seem to
be called, and it wouldn't be appropriate to call them because they just
throw. Being pure, they can't get called because we neglected to implement
them for some derived class: Derived::foo() would have to call Base::foo()
explicitly. So they're useless.

Interestingly, though, this example shows that, given Base::foo()=0, if any
Derived::foo() returns, then an implementation of Base::foo() cannot be
marked [[noreturn]] even if it cannot return...so the language is acquiring
more weird corner cases. I just read that "[[" is forbidden except as part
of an attribute-specifier--7.6.1/6 says this
  y[[] { return 2; }()] = 2;
is an error even though it couldn't possibly be an attribute. We just got
the ability to write ">>" in "template<T, U<T>>", and now we've lost the
(admittedly less useful) "[[". Maybe Rust can be cleaner and smaller.