946791 - GLContextGLX::mLibType is no longer used

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler]]

0:07.50 /home/keeler/mozilla-central/gfx/gl/GLContextProviderGLX.cpp:970:13: error: private field 'mLibType' is not used [-Werror,-Wunused-private-field]

This is with clang. It looks like bug 942501 removed the last instance of its use (as far as I can tell - there may be something more subtle going on): http://hg.mozilla.org/mozilla-central/diff/93d12e87d7d6/gfx/gl/GLContextProviderGLX.cpp#l1.195

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: patch

This fixes it for me, but again, I'm not entirely sure something more subtle isn't going on (e.g. was it intended that this no longer be used?)
Anyway, r? to Benoit.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

There are more occurences of mLibType in this file than just these two ones, so I am unsure how this patch compiles. Or does it apply to a different tree?

It is true that mLibType is not useful at the moment, and that it would be a good thing to remove it. It was meant to allow switching between system OpenGL, and a custom build of llvmpipe that we'd ship with Firefox as a software fallback for WebGL rendering. That never happened. It's OK to drop this code now. We have a better plan to implement such things going forward anyway, should this topic be brought up again.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 8343192 [details] [diff] [review]
patch

r- f+ for reasons explained in above comment.

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler]]

(In reply to Benoit Jacob [:bjacob] from comment #2)
> There are more occurences of mLibType in this file than just these two ones,
> so I am unsure how this patch compiles. Or does it apply to a different tree?

All other uses are the mLibType in GLXLibrary. This patch removes the occurrences in GLContextGLX.
This is on mozilla-central.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 8343192 [details] [diff] [review]
patch

Oh, sorry, I got confused. Thanks.

Comment 6

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Great - thanks!
(Looking back at that patch, there's not enough context to figure out that GLXLibrary::mLibType is being left alone, so I added that to the commit message.)
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fd483b8cf66

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6fd483b8cf66