1373625 - warning: lambda capture 'this' is not used [-Wunused-lambda-capture]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

In file included from /home/sylvestre/dev/mozilla/mozilla-central.hg/obj-x86_64-pc-linux-gnu/layout/generic/Unified_cpp_layout_generic1.cpp:74:
/home/sylvestre/dev/mozilla/mozilla-central.hg/layout/generic/nsContainerFrame.cpp:336:24: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
   mProperties.ForEach([this, aLists] (const T& aProp, void* aValue) {
                        ^

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1373625 - Silence a -Wunused-lambda-capture warning in nsContainerFrame.cpp

Review commit: https://reviewboard.mozilla.org/r/149808/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/149808/

Comment 2

[Mats Palmgren (inactive)]

It's needed for the IsFrameOfType() call in the MOZ_ASSERT,
so I don't think we can just remove it.

Comment 3

[Mats Palmgren (inactive)]

Perhaps we could make an exception and do a "capture all you need" in this case?
[=] I guess since they are both pointers.

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 8878457 [details]
Bug 1373625 - Silence a -Wunused-lambda-capture warning in nsContainerFrame.cpp

https://reviewboard.mozilla.org/r/149808/#review154564

r- on current patch, since (as mats notes) it won't compile with ac_add_options --enable-debug, due to the MOZ_ASSERT usage of "this".

If we really want to silence the warning, we could maybe just do...
  Unused << this; // silence clang -Wunused-lambda-capture in opt builds
...right after the MOZ_ASSERT? *shrug*

Comment 5

[Mats Palmgren (inactive)]

Sure, "Unused << this" works for me.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8878457 [details]
Bug 1373625 - Silence a -Wunused-lambda-capture warning in nsContainerFrame.cpp

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149808/diff/1-2/

Comment 7

[Sylvestre Ledru [:Sylvestre]]

voila!

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 8878457 [details]
Bug 1373625 - Silence a -Wunused-lambda-capture warning in nsContainerFrame.cpp

https://reviewboard.mozilla.org/r/149808/#review156344

r=me with two nits:

::: commit-message-fe809:1
(Diff revision 2)
> +Bug 1373625 - Silent a -Wunused-lambda-capture warning in nsContainerFrame.cpp r?dholbert

s/Silent/Silence/

::: layout/generic/nsContainerFrame.cpp:351
(Diff revision 2)
>                   "found unexpected ExcessOverflowContainersProperty");
>        L(aValue)->AppendIfNonempty(aLists, kExcessOverflowContainersList);
>      } else if (aProp == BackdropProperty()) {
>        L(aValue)->AppendIfNonempty(aLists, kBackdropList);
>      }
> +    Unused << this; // silence clang -Wunused-lambda-capture in opt builds

Please put this new line *directly after* the MOZ_ASSERT, so that the debug-build-usage & the opt-build-hackaround are more clearly associated.

(And if we happen to remove the MOZ_ASSERT someday, we'll be more likely to realize that we should drop the Unused<< as well, and then the compiler can help us figure out to drop the [this] lambda capture.)

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8878457 [details]
Bug 1373625 - Silence a -Wunused-lambda-capture warning in nsContainerFrame.cpp

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149808/diff/2-3/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85c7a1f9a5b4
Silence a -Wunused-lambda-capture warning in nsContainerFrame.cpp r=dholbert

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/85c7a1f9a5b4