1601843 - (maybe_unused) Replace MOZ_MAYBE_UNUSED macro with C++17 attribute [[maybe_unused]]

Status: ASSIGNED

(Core :: MFBT, task, P3)

Description

[Masatoshi Kimura [:emk]]

Like bug 1571631 and bug 1570499.

Comment 1

[Chris Peterson [:cpeterson]]

MOZ_MAYBE_UNUSED should be easy to replace because there are few uses. That assumes [[maybe_unused]] has the same behavior as MOZ_MAYBE_UNUSED's __attribute__((__unused__)) (which is not the case with [[nodiscard]] and MOZ_MUST_USE's __attribute__((warn_unused_result)) in bug 1571631).

https://searchfox.org/mozilla-central/search?case=true&q=MOZ_MAYBE_UNUSED

Comment 2

[NicolasPacheco]

(In reply to Chris Peterson [:cpeterson] from comment #1)

MOZ_MAYBE_UNUSED should be easy to replace because there are few uses. That assumes [[maybe_unused]] has the same behavior as MOZ_MAYBE_UNUSED's __attribute__((__unused__)) (which is not the case with [[nodiscard]] and MOZ_MUST_USE's __attribute__((warn_unused_result)) in bug 1571631).

https://searchfox.org/mozilla-central/search?case=true&q=MOZ_MAYBE_UNUSED
'
Hey Chris, this bug is just changing MOZ_MAYBE_UNUSED by [[maybe_unused]] ? I would like to solve it. Do I need more information or is it just that?

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to NicolasPacheco from comment #2)

Hey Chris, this bug is just changing MOZ_MAYBE_UNUSED by [[maybe_unused]] ? I would like to solve it. Do I need more information or is it just that?

Hi Nicolas! Thanks for your help on this bug. Yes, the steps to fix this bug are:

1. Replace uses of the MOZ_MAYBE_UNUSED macro with the [[maybe_unused]] attribute.
2. Remove the MOZ_MAYBE_UNUSED macro definition in mfbt/Attributes.h.

Have you built and run Firefox on your computer yet? That's the first step. Here are build instructions:

https://developer.mozilla.org/docs/Mozilla/Developer_guide/Build_Instructions

Note that switching to [[maybe_unused]] may produce some new warnings or errors that will need to be fixed. Even "simple" find-and-replace bugs like this have a way of revealing "interesting" dependencies. :) For example, I expect MOZ_MAYBE_UNUSED and [[maybe_unused]] may have different requirements for how they are ordered in function declarations. I saw this when replacing the MOZ_MUST_USE macro with the [[nodiscard]] attribute.

So a function declaration like:

static MOZ_MAYBE_UNUSED void SaveOncePrefToSharedMap(...

may need to be rearranged like:

[[maybe_unused]] static void SaveOncePrefToSharedMap(...

Comment 4

[NicolasPacheco]

Yes!(In reply to Chris Peterson [:cpeterson] from comment #3)

(In reply to NicolasPacheco from comment #2)

Hey Chris, this bug is just changing MOZ_MAYBE_UNUSED by [[maybe_unused]] ? I would like to solve it. Do I need more information or is it just that?

Hi Nicolas! Thanks for your help on this bug. Yes, the steps to fix this bug are:

1. Replace uses of the MOZ_MAYBE_UNUSED macro with the [[maybe_unused]] attribute.
2. Remove the MOZ_MAYBE_UNUSED macro definition in mfbt/Attributes.h.

Have you built and run Firefox on your computer yet? That's the first step. Here are build instructions:
Yes! I have already work with some bugs. I will at look in this one!
https://developer.mozilla.org/docs/Mozilla/Developer_guide/Build_Instructions

Note that switching to [[maybe_unused]] may produce some new warnings or errors that will need to be fixed. Even "simple" find-and-replace bugs like this have a way of revealing "interesting" dependencies. :) For example, I expect MOZ_MAYBE_UNUSED and [[maybe_unused]] may have different requirements for how they are ordered in function declarations. I saw this when replacing the MOZ_MUST_USE macro with the [[nodiscard]] attribute.

So a function declaration like:

static MOZ_MAYBE_UNUSED void SaveOncePrefToSharedMap(...

may need to be rearranged like:

[[maybe_unused]] static void SaveOncePrefToSharedMap(...

Comment 5

[Chris Peterson [:cpeterson]]

Awesome. If you need help uploading a patch to Phabricator for code review, just let me know. I can pre-review of your patch before we ask for an official review.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:sg, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 7

[Andi [:andi]]

Not sure we can do this since thee are include headers, like Assertions.h that are included from c files and this attributes, [[maybe_unused]] I think are available from C23.

Comment 8

[Jake Senne]

Attachment: Bug 1601843 - Replace MOZ_MAYBE_UNUSED with [[maybe_unused]]. r?#dom-storage,#spidermonkey-reviewers,#xpcom-reviewers

Comment 9

[Chris Peterson [:cpeterson]]

(In reply to Andi [:andi] from comment #7)

[[maybe_unused]] I think are available from C23.

That's correct. C23 is the first C version that supports [[maybe_unused]]:

https://en.cppreference.com/w/c/language/attributes/maybe_unused