Page MenuHomeFreeBSD
Differential D35959

style: Tighten up one use of 'may'
ClosedPublic
Actions

Authored by imp on Jul 26 2022, 8:23 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 16 2024, 2:38 PM
Unknown Object (File)
Feb 6 2024, 3:17 AM
Unknown Object (File)
Jan 30 2024, 5:53 PM
Unknown Object (File)
Jan 30 2024, 5:53 PM
Unknown Object (File)
Jan 30 2024, 5:53 PM
Unknown Object (File)
Jan 30 2024, 5:53 PM
Unknown Object (File)
Jan 22 2024, 11:06 AM
Unknown Object (File)
Dec 18 2023, 7:41 PM
View All 45 Files
Subscribers
cperciva
emaste
• hselasky
jhb
pauamma_gundo.com

Details

Reviewers
pstef
rpokala
• hselasky
Group Reviewers
manpages
Commits
rGe2fa10e67646: style: Tighten up one use of 'may'
Summary

Declarations of variables must be placed before the executable lines of
a block, by convention. Use 'must' instead of 'may' here and clarify wording.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 46616
Build 43505: arc lint + arc unit

Event Timeline

imp created this revision.Jul 26 2022, 8:23 PM
Herald added a reviewer: manpages. · View Herald TranscriptJul 26 2022, 8:23 PM
imp requested review of this revision.Jul 26 2022, 8:23 PM
Harbormaster completed remote builds in B46610: Diff 108572.Jul 26 2022, 8:23 PM
pstef accepted this revision.Jul 26 2022, 8:23 PM
This revision is now accepted and ready to land.Jul 26 2022, 8:23 PM
rpokala accepted this revision.Jul 26 2022, 8:24 PM
cperciva added a subscriber: cperciva.Jul 26 2022, 8:25 PM
cperciva added inline comments.
share/man/man9/style.9
703–704

I think the "may" refers to the "at the start of any block". i.e. what is intended is "Declarations must be placed before executable lines, and may occur at the start of any block."

imp updated this revision to Diff 108573.Jul 26 2022, 8:28 PM

Improve with Collin's suggestion

This revision now requires review to proceed.Jul 26 2022, 8:28 PM
Harbormaster completed remote builds in B46611: Diff 108573.Jul 26 2022, 8:28 PM
pstef accepted this revision.Jul 26 2022, 8:39 PM
pstef added inline comments.
share/man/man9/style.9
703–704

I think this is a good suggestion.
I would go further and replace "executable lines" with "statements" to follow the clang message.

This revision is now accepted and ready to land.Jul 26 2022, 8:39 PM
emaste added a subscriber: emaste.Jul 26 2022, 8:50 PM
emaste added inline comments.
share/man/man9/style.9
703–704

I like pstef's suggestion.

emaste added inline comments.Jul 26 2022, 8:56 PM
share/man/man9/style.9
538

We have only 307 NOTREACHEDs in the tree, in 121 files. annotations (like __dead2 or __assert_unreachable as some folks mentioned on IRC) are much better

imp updated this revision to Diff 108575.Jul 26 2022, 9:00 PM

__assert_unreachable is the more modern spelling of /* NOTREACHED */ so
recommend that. Use statement (the language the C standard uses) rather than the
more vague executable lines.

This revision now requires review to proceed.Jul 26 2022, 9:00 PM
Harbormaster completed remote builds in B46612: Diff 108575.Jul 26 2022, 9:00 PM
imp updated this revision to Diff 108577.Jul 26 2022, 9:06 PM

mjg points out this isn't in userland.

Harbormaster completed remote builds in B46614: Diff 108577.Jul 26 2022, 9:06 PM
rpokala added inline comments.Jul 26 2022, 9:13 PM
share/man/man9/style.9
538
Code which is unreachable for non-obvious reasons should be marked accordingly.
In the kernel,
.Fn __assert_unreachable
should be used in preference to the older /*
.Li NOTREACHED
*/.
That annotation is not available in userland,
so use /*
.Li NOTREACHED
*/ there instead.
jhb added a subscriber: jhb.Jul 26 2022, 9:22 PM
jhb added inline comments.
share/man/man9/style.9
538

I'd rather just expose __assert_unreachable in userland.

I don't read the previous version as encouraging the use of annotations, just permitting them. The new version reads to me as if it more strongly mandates it. That is, I feel like the old use of may here was actually correct.

imp added inline comments.Jul 26 2022, 9:41 PM
share/man/man9/style.9
538

I think I'll drop these chunks of the change as 'premature' then. We'll revisit once we have different interfaces and want a stronger encouragement.

imp retitled this revision from style: Tighten up some uses of 'may' to style: Tighten up one use of 'may'.Jul 26 2022, 9:47 PM
imp edited the summary of this revision. (Show Details)
imp updated this revision to Diff 108579.Jul 26 2022, 9:47 PM

drop notreached... it's not quite ready

Harbormaster completed remote builds in B46616: Diff 108579.Jul 26 2022, 9:47 PM
imp marked 6 inline comments as done.Jul 26 2022, 9:48 PM
pauamma_gundo.com added a subscriber: pauamma_gundo.com.Jul 27 2022, 2:23 PM

Worth adding a short description of must/should/may/should not/must not a la RFC2119?

share/man/man9/style.9
2
28

Bump.

41
• hselasky accepted this revision.Jul 28 2022, 1:57 PM
• hselasky added a subscriber: • hselasky.

No objections.

This revision is now accepted and ready to land.Jul 28 2022, 1:57 PM
Closed by commit rGe2fa10e67646: style: Tighten up one use of 'may' (authored by imp). · Explain WhyJul 28 2022, 2:06 PM
This revision was automatically updated to reflect the committed changes.
imp added a commit: rGe2fa10e67646: style: Tighten up one use of 'may'.
imp added a comment.Jul 28 2022, 2:07 PM

So that's a 'no' on expanding the scope of this review to have full RFC-language. I may do that in the future, but I don't
have time for it now.

Revision Contents
Changeset List

PathSize
share/
man/
man9/
3 lines

Diff 108579

View Options

share/man/man9/style.9

Loading...