Merge lp:~vanvugt/mir/remove-nonexistent-ClientBuffers-symbols into lp:mir

[Daniel van Vugt]

src/server/symbols.map: Remove non-existent symbol
   mir::frontend::ClientBuffers::operator*;

It was removed in r4064, but even before then never really
existed as an addressable symbol to be versioned.

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:4063
https://mir-jenkins.ubuntu.com/job/mir-ci/3093/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4142
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4229
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4219
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4219
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4219
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4169
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4169/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4169
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4169/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4169
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4169/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4169
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4169/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4169
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4169/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4169
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4169/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3093/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

"It's pure virtual so has no address to export."

That may be true in this case, but isn't true in general. It is perfectly reasonable to implement a pure virtual function and to call it. That's why the script allows it to be exported.

review: Abstain

[Daniel van Vugt (vanvugt)]

You are either referring to an implementation in a different class, or referring to a non-pure virtual method. Yes, both of those cases would generate code with a symbol to export. But a pure virtual method never exists as a named binary symbol.

I think we could reliably modify the script to omit these, although not a major priority given it's arguably a useful hint in symbols.map that such non-existent symbols do indirectly affect the ABI still (via the vtable). I think I just argued against this branch... but still it's useful that we all have an idea of what's going on.

On the other hand, there is no functional reason for non-existent symbols to be listed for versioning. If you acknowledge that symbols.map is not a definitive list of things that might break your ABI then we don't need to put unused strings in there.

[Alan Griffiths (alan-griffiths)]

> You are either referring to an implementation in a different class, or
> referring to a non-pure virtual method. Yes, both of those cases would
> generate code with a symbol to export. But a pure virtual method never exists
> as a named binary symbol.

struct Base { virtual void pure_virtual() = 0; };

struct Derived : Base { void pure_virtual() override; };

// implementing
void Base::pure_virtual() { }

// calling
void Derived::pure_virtual() { Base::pure_virtual(); }

[Daniel van Vugt (vanvugt)]

Oh my. It seems you can implement a pure virtual method within the class where it is pure virtual. I did not expect that. Thanks.

Although it is now a moot point. Revision 4064 deleted the operator so it still needs to be deleted from symbols.map :)

[Cemil Azizoglu (cemil-azizoglu)]

ok

review: Approve

=== modified file 'src/server/symbols.map'
--- src/server/symbols.map	2017-02-16 05:44:15 +0000
+++ src/server/symbols.map	2017-03-02 10:11:15 +0000
@@ -37,7 +37,6 @@
     mir::frontend::BufferStream::operator*;
     mir::frontend::ClientBuffers::?ClientBuffers*;
     mir::frontend::ClientBuffers::ClientBuffers*;
-    mir::frontend::ClientBuffers::operator*;
     mir::frontend::PromptSession::operator*;
     mir::frontend::PromptSession::?PromptSession*;
     mir::frontend::PromptSession::PromptSession*;