This is an archive of the discontinued LLVM Phabricator instance.

[libc++][format] Make public functions nodiscard.
ClosedPublic

Authored by Mordante on Jun 4 2023, 8:35 AM.

Details

Reviewers
philnik
Group Reviewers
Restricted Project
Commits
rG9c053e69939b: [libc++][format] Make public functions nodiscard.
Summary

This is an extension and only adds the functions that are a considered a
but when called and ignoring the result.

Drive-by sort all nodiscard extensions in the documentation.

Diff Detail

Event Timeline

Mordante created this revision.Jun 4 2023, 8:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2023, 8:35 AM
Mordante requested review of this revision.Jun 4 2023, 8:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2023, 8:35 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante updated this revision to Diff 528243.Jun 4 2023, 11:25 AM

CI fixes.

philnik requested changes to this revision.Jun 8 2023, 10:02 AM
philnik added a subscriber: philnik.
philnik added inline comments.
libcxx/test/libcxx/diagnostics/format.nodiscard_extensions.compile.pass.cpp
3

You seem to be missing a positive test.

libcxx/test/std/utilities/format/format.arguments/format.arg.store/make_format_args.sh.cpp
27–28

Why not simply cast to void?

This revision now requires changes to proceed.Jun 8 2023, 10:02 AM
Mordante marked 2 inline comments as done.Jun 10 2023, 6:33 AM
Mordante added a subscriber: BillyONeal.

Thanks for the review!

libcxx/test/libcxx/diagnostics/format.nodiscard_extensions.compile.pass.cpp
3

Nice catch, I'll commit my local file this time ;-)

libcxx/test/std/utilities/format/format.arguments/format.arg.store/make_format_args.sh.cpp
27–28

I prefer name instead of a magic invocation, @BillyONeal spend the effort to change them in D40065.

Mordante updated this revision to Diff 530203.Jun 10 2023, 6:34 AM
Mordante marked 2 inline comments as done.

Addresses review comments.

philnik accepted this revision.Jun 12 2023, 8:04 AM
philnik added inline comments.
libcxx/test/std/utilities/format/format.arguments/format.arg.store/make_format_args.sh.cpp
27–28

IMO it's kind-of pointless to have a macro that just casts to void, since its people know what casting to void means, but not what a given macro actually does. It looks to me like the macro does something magical to check that it's actually ignoring nodiscard.

This revision is now accepted and ready to land.Jun 12 2023, 8:04 AM

Thanks for the review!

libcxx/test/std/utilities/format/format.arguments/format.arg.store/make_format_args.sh.cpp
27–28

I like he macro since it explains why it's needed.

This revision was automatically updated to reflect the committed changes.