Replace MOZ_MUST_USE with [[nodiscard]] in widget
Categories
(Core :: Widget, task, P3)
Tracking
()
People
(Reporter: cpeterson, Assigned: cpeterson)
References
(Blocks 1 open bug)
Details
Attachments
(6 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
The MOZ_MUST_USE macro is defined as clang's and gcc's nonstandard attribute((warn_unused_result)). Now that we compile as C++17 by default (bug 1560664), we can replace MOZ_MUST_USE with C++17's standard [[nodiscard]] attribute.
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's __attribute__((warn_unused_result))
can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's attribute((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Assignee | ||
Comment 2•5 years ago
|
||
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's attribute((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Depends on D68146
Assignee | ||
Comment 3•5 years ago
|
||
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's attribute((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Depends on D68147
Assignee | ||
Comment 4•5 years ago
|
||
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's attribute((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Depends on D68148
Assignee | ||
Comment 5•5 years ago
|
||
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's attribute((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Depends on D68149
Assignee | ||
Comment 6•5 years ago
|
||
leave-open because I'm landing patches #1 and #2 so they don't bitrot while waiting for feedback on patch #3.
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/beb748e23620 Replace MOZ_MUST_USE with [[nodiscard]] in widget and widget/windows. r=jmathies https://hg.mozilla.org/integration/autoland/rev/b21157048591 Replace MOZ_MUST_USE with [[nodiscard]] in widget/android. r=snorp
Comment 8•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/beb748e23620
https://hg.mozilla.org/mozilla-central/rev/b21157048591
Assignee | ||
Comment 9•5 years ago
|
||
nit: The indent seems off here.
Stephen, following up on your comment in https://phabricator.services.mozilla.com/D68148#inline-404257. Unfortunately, hg's clang-format hook forces this indentation (unless I add a superfluous semicolon after NS_DECL_NSPIWIDGETCOCOA
).
Which of these options do you prefer?
- Leave clang-format's extra indentation level.
- Add a superfluous semicolon after
NS_DECL_NSPIWIDGETCOCOA
, though that might trigger a compiler warning about the extra semicolon. (TheNS_DECL_NSPIWIDGETCOCOA
macro definition already includes a semicolon.) - Wrap this
Create()
function declaration in// clang-format off
and// clang-format on
to disable clang-format for that block of code.
Assignee | ||
Comment 10•5 years ago
|
||
I'm plan to:
- Wrap the Create() function declaration in
// clang-format off
and// clang-format on
to disable clang-format for that block of code. - Ask the linting team whether this is clang-format bug or setting we can fix.
Assignee | ||
Comment 11•5 years ago
•
|
||
Sylvetre, is the problem below a clang-format bug or setting we can work around?
clang-format insists on indenting the first Create()
function declaration here:
https://phabricator.services.mozilla.com/D68148#change-ItZetrh2GZ7W
class nsCocoaWindow final : public nsBaseWidget, public nsPIWidgetCocoa {
...
NS_DECL_ISUPPORTS_INHERITED
NS_DECL_NSPIWIDGETCOCOA
[[nodiscard]] virtual nsresult Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
const DesktopIntRect& aRect,
nsWidgetInitData* aInitData = nullptr) override;
[[nodiscard]] virtual nsresult Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
const LayoutDeviceIntRect& aRect,
nsWidgetInitData* aInitData = nullptr) override;
clang-format mistakenly thinks the NS_DECL_NSPIWIDGETCOCOA
macro a couple lines before the [[nodiscard]]
should be part of the Create()
function declaration. The NS_DECL_NSPIWIDGETCOCOA
macro injects some function declarations should not impact the indentation level of surrounding code. I feel this is a clang-format bug.
https://searchfox.org/mozilla-central/source/__GENERATED__/dist/include/nsPIWidgetCocoa.h#52-57
To work around this, I have to add:
class nsCocoaWindow final : public nsBaseWidget, public nsPIWidgetCocoa {
...
// clang-format off
NS_DECL_ISUPPORTS_INHERITED
NS_DECL_NSPIWIDGETCOCOA
[[nodiscard]] virtual nsresult Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
const DesktopIntRect& aRect,
nsWidgetInitData* aInitData = nullptr) override;
// clang-format on
[[nodiscard]] virtual nsresult Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
const LayoutDeviceIntRect& aRect,
nsWidgetInitData* aInitData = nullptr) override;
Comment 12•5 years ago
|
||
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/81e4b5376ea8 Replace MOZ_MUST_USE with [[nodiscard]] in widget/cocoa. r=spohl https://hg.mozilla.org/integration/autoland/rev/c4832a04688a Replace MOZ_MUST_USE with [[nodiscard]] in widget/gtk. r=karlt https://hg.mozilla.org/integration/autoland/rev/9e2d7aa1942c Replace MOZ_MUST_USE with [[nodiscard]] in widget/headless. r=bdahl
Assignee | ||
Updated•5 years ago
|
Comment 13•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/81e4b5376ea8
https://hg.mozilla.org/mozilla-central/rev/c4832a04688a
https://hg.mozilla.org/mozilla-central/rev/9e2d7aa1942c
Assignee | ||
Comment 14•4 years ago
|
||
This semicolon workaround is tidier than the // clang-format off/on
comments and avoids turning off all clang-format checks. The comment also links to clang-format bug 1629756 so future code readers can learn why this extra semicolon exists. And if we find a way to fix this in clang-format, then we can search for this bug number to find and remove these extra semicolons and comments.
Comment 15•4 years ago
|
||
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a6637962605f Replace "clang-format off" with semicolon workaround for clang-format bug 1629756. r=spohl
Comment 16•4 years ago
|
||
bugherder |
Comment 17•4 years ago
|
||
Sorry for the latency, I think it is now fixed upstream
Description
•