This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Use [[nodiscard]] for lock_guard, as an extension
ClosedPublic

Authored by ldionne on Aug 7 2019, 1:02 PM.

Details

Summary

D64914 added support for applying [[nodiscard]] to constructors. This
commit uses that capability to flag incorrect uses of std::lock_guard
where one forgets to actually create a variable for the lock_guard.

rdar://45790820

Diff Detail

Repository
rL LLVM

Event Timeline

ldionne created this revision.Aug 7 2019, 1:02 PM
ldionne updated this revision to Diff 214155.Aug 8 2019, 7:53 AM

Update the list of entities to which we apply [[nodiscard]]

EricWF accepted this revision.Aug 8 2019, 11:56 AM
EricWF added inline comments.
libcxx/include/__config
1016 ↗(On Diff #214155)

What happens when we apply this attribute to a constructor with a compiler that doesn't yet support it?

Does the warning escape the system header?

This revision is now accepted and ready to land.Aug 8 2019, 11:56 AM
xbolva00 added inline comments.
libcxx/include/__config
1016 ↗(On Diff #214155)

If compiler returns >= 201907L, it have to support nodiscard on ctors.

Hopefully no compiler just bumps it to 201907L and does not support it.

ldionne marked an inline comment as done.Aug 8 2019, 1:00 PM
ldionne added inline comments.
libcxx/include/__config
1016 ↗(On Diff #214155)

What happens when we apply this attribute to a constructor with a compiler that doesn't yet support it?

<stdin>:2:14: warning: attribute 'nodiscard' cannot be applied to functions without return value [-Wignored-attributes]

Does the warning escape the system header?

No, which means we could just use _LIBCPP_NODISCARD/_LIBCPP_NODISCARD_EXT on the constructor and let the warning be captured inside the system header. That's what you had in mind to avoid introducing another macro, right? Would you prefer that?

ldionne updated this revision to Diff 214649.Aug 12 2019, 9:48 AM

Avoid introducing a special macro for [[nodiscard]] on constructors.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2019, 4:12 AM