1369622 - Followups after bug 1368932

Status: RESOLVED FIXED

(Core :: MFBT, enhancement)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1369622 - Fix MOZ_FOR_EACH with an empty list.

I'm not sure how I tested MOZ_FOR_EACH in bug 1368932, but it turns out
it doesn't work with an empty list, despite
MOZ_PASTE_PREFIX_AND_ARG_COUNT now supporting 0 arguments.

Macros can be tricky, and it ends up being easier to make things work
cross-compiler with a separate macro that does the counting, and
(re)building MOZ_PASTE_PREFIX_AND_ARG_COUNT on top of that. Then
MOZ_FOR_EACH ends up working as expected with an empty list.

So this adds a MOZ_ARG_COUNT macro that counts the number of variadic
arguments it's given, and derives MOZ_PASTE_PREFIX_AND_ARG_COUNT from
it.

And this adds a testcase validating that MOZ_FOR_EACH works properly
with an empty list as a result.

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

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1369622 - Use MOZ_ARG_COUNT instead of MOZ_PASTE_PREFIX_AND_ARG_COUNT.

In a couple places, MOZ_PASTE_PREFIX_AND_ARG_COUNT is used to only count
the number of arguments, we can now use MOZ_ARG_COUNT directly for that.

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

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1369622 - Restore static asserts for lack of arguments on some macros.

Bug 1368932 removed MOZ_STATIC_ASSERT_VALID_ARG_COUNT because it didn't
actually work for large numbers of arguments. But it was kind of useful
for macros that expand to something broken when they are given no
variadic argument at all.

Now that we have a macro that doesn't require tricks to count empty
arguments lists, however, we can use straightforward static_asserts
instead of a generic macro, which has the side effect of providing more
context in the error message.

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

Comment 4

[Mike Hommey [:glandium]]

Attachment: Bug 1369622 - Add a static_assert when no variadic argument is given to MOZ_CRASH_UNSAFE_PRINTF.

If MOZ_CRASH_UNSAFE_PRINTF is only given a format string, it means
either arguments are missing, or MOZ_CRASH should be used instead.
Hint at that with a static_assert.

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

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8873715 [details]
Bug 1369622 - Fix MOZ_FOR_EACH with an empty list.

https://reviewboard.mozilla.org/r/145106/#review150208

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8873716 [details]
Bug 1369622 - Use MOZ_ARG_COUNT instead of MOZ_PASTE_PREFIX_AND_ARG_COUNT.

https://reviewboard.mozilla.org/r/145108/#review150210

Comment 7

[Nathan Froyd [:froydnj]]

Comment on attachment 8873717 [details]
Bug 1369622 - Restore static asserts for lack of arguments on some macros.

https://reviewboard.mozilla.org/r/145110/#review150214

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 8873718 [details]
Bug 1369622 - Add a static_assert when no variadic argument is given to MOZ_CRASH_UNSAFE_PRINTF.

https://reviewboard.mozilla.org/r/145112/#review150216

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bee4404d2f88
Fix MOZ_FOR_EACH with an empty list. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/e51f693cdbd3
Use MOZ_ARG_COUNT instead of MOZ_PASTE_PREFIX_AND_ARG_COUNT. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/f360dd9d411e
Restore static asserts for lack of arguments on some macros. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/e1a83fc8ff4f
Add a static_assert when no variadic argument is given to MOZ_CRASH_UNSAFE_PRINTF. r=froydnj

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/bee4404d2f88
https://hg.mozilla.org/mozilla-central/rev/e51f693cdbd3
https://hg.mozilla.org/mozilla-central/rev/f360dd9d411e
https://hg.mozilla.org/mozilla-central/rev/e1a83fc8ff4f