Closed Bug 1345074 Opened 3 years ago Closed 3 years ago

Add an alternative form of MOZ_FOR_EACH for having separator between items

Categories

(Core :: MFBT, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

Details

Attachments

(1 file)

So far we have MOZ_FOR_EACH, and if we have (a, b, c) and want to make it a + b + c, we can do
> #define MACRO_A(x) x +
> MOZ_FOR_EACH(MACRO_A, (), (a, b, c)) 0

However, it would be better if we don't have the trailing zero.

In addition, it is very hard for MOZ_FOR_EACH to generate param list of functions, e.g. if we have (a, b, c) and want to generate |void test(int a, int b, int c)|, because in this case, trailing comma is not allowed.

So it would be useful to have a help macro which can insert separator between items, so that we can do something like
> #define MACRO_A(x) int x
> void test(MOZ_FOR_EACH2(MACRO_A, (,), (), (a, b, c)));
Assignee: nobody → xidorn+moz
Comment on attachment 8844404 [details]
Bug 1345074 - Add MOZ_FOR_EACH_SEPARATED which adds separator between items.

https://reviewboard.mozilla.org/r/117890/#review119672

::: mfbt/MacroForEach.h:38
(Diff revision 1)
>   *   #define MACRO_C(k1, k2, x) (k1 + k2 + x) +
>   *   int c = MOZ_FOR_EACH(MACRO_C, (5, 8,), (1, 2)) 0;
>   *   // Expands to: MACRO_B(5, 8, 1) MACRO_B(5, 8, 2) 0;
>   *
> + * MOZ_FOR_EACH2(aMacro, aSeparator, aFixedArgs, aArgs) is identical to
> + * MOZ_FOR_EACH except that it inserts |aSeparator| between each call to

This is bad naming.  A "2" name implies an improved version of an original, ultimately to supplant it.  But it's quite plausible that users will want both of these, e.g. the separator variety for argument lists and the non-separator variety for something like generating the |static const char foo[] = "foo";| bits the JS engine uses for named constants for keyword strings.

Let's have the existing MOZ_FOR_EACH with that name, and this new one as MOZ_FOR_EACH_SEPARATED.  Seem reasonable?

::: mfbt/MacroForEach.h:49
(Diff revision 1)
> + *   // And further to: 1 + 2 + 3
> + *
> + *  #define MACRO_B(t, n) t n
> + *  void test(MOZ_FOR_EACH2(MACRO_B, (,), (int), (a, b)));
> + *  // Expands to: void test(MACRO_B(int, a) , MACRO_B(int, b));
> + *  // And further to: void test(int a , int b);

This second code block should be indented consistent with the one before it: two spaces from the left column for the paragraphs in the overall comment.

::: mfbt/MacroForEach.h:60
(Diff revision 1)
>   * results in a compile-time error.
>   */
>  #define MOZ_FOR_EACH_EXPAND_HELPER(...) __VA_ARGS__
>  #define MOZ_FOR_EACH_GLUE(a, b) a b
>  #define MOZ_FOR_EACH(aMacro, aFixedArgs, aArgs) \
> +  MOZ_FOR_EACH2(aMacro, (), aFixedArgs, aArgs)

I'd prefer if MOZ_FOR_EACH were defined *after* MOZ_FOR_EACH_SEPARATED, because its definition relies on that macro.  Maybe the preprocessor doesn't quite work that way actually, but it's nicer to be able to think of it as doing so.
Attachment #8844404 - Flags: review?(jwalden+bmo) → review-
Comment on attachment 8844404 [details]
Bug 1345074 - Add MOZ_FOR_EACH_SEPARATED which adds separator between items.

https://reviewboard.mozilla.org/r/117890/#review119858
Attachment #8844404 - Flags: review?(jwalden+bmo) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9d30ac5bca6e
Add MOZ_FOR_EACH_SEPARATED which adds separator between items. r=Waldo
https://hg.mozilla.org/mozilla-central/rev/9d30ac5bca6e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.