This is an archive of the discontinued LLVM Phabricator instance.

[docs] Better documentation for -Weverything
ClosedPublic

Authored by jfb on Aug 3 2019, 3:26 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

jfb created this revision.Aug 3 2019, 3:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2019, 3:26 PM
lebedev.ri added inline comments.
clang/docs/UsersManual.rst
998 ↗(On Diff #213210)

users *of*

jfb updated this revision to Diff 213212.Aug 3 2019, 3:30 PM
  • Missing 'of'
jfb marked an inline comment as done.Aug 3 2019, 3:30 PM
xbolva00 retitled this revision from [docs] document -Weveything more betterer to [docs] Better documentation for -Weverything.Aug 3 2019, 3:43 PM

Thank you for working on this!

clang/docs/UsersManual.rst
998 ↗(On Diff #213212)

users of -Weverything therefore -> therefore, users of -Weverything

999–1000 ↗(On Diff #213212)

Would you care to propose a more exhaustive list of conflicting diagnostics? (Perhaps in a follow-up patch.)

1006 ↗(On Diff #213212)

once -> ones
Add a comma after -Weverything

1008 ↗(On Diff #213212)

clang -> Clang

jfb updated this revision to Diff 213278.Aug 4 2019, 8:33 PM
jfb marked 5 inline comments as done.
  • Address Aaron's comments.
clang/docs/UsersManual.rst
999–1000 ↗(On Diff #213212)

I looked a bit and I'm worried that providing a list won't be particularly satisfying for people looking at this. I think it's better to have some warning, and let folks figure out what works for their particular situation. Here I'm assuming that they don't use C++98 and that seems reasonable, but figuring out what side of contradictions they're on doesn't seem like it'll work out.

aaron.ballman added inline comments.Aug 5 2019, 5:58 AM
clang/docs/UsersManual.rst
999–1000 ↗(On Diff #213212)

One of the primary concerns with enabling -Weverything is the fact that we know this enables conflicting diagnostics. Telling the user "we know there are conflicting diagnostics, but we want you to have the joy of figuring out which ones conflict for yourself" seems even more unsatisfying, to me. I agree that we don't want to tell users which of the conflicting options they should disable, but was thinking of something more along the lines of:

The following sets of options are known to have some possibly unfortunate interactions when enabled together:
  * -Wfoo, -Wbar
  * -Wbaz, -Wquux
  * ...
Note that there may be other conflicting diagnostic flags not listed above.

I figure that gives users a bit more of an idea of what they're signing up for when they enable -Weverything, which seems useful.

jfb marked an inline comment as done.Aug 5 2019, 9:38 AM
jfb added inline comments.
clang/docs/UsersManual.rst
999–1000 ↗(On Diff #213212)

OK I can do that as a follow-up.

aaron.ballman accepted this revision.Aug 5 2019, 9:50 AM

LGTM!

clang/docs/UsersManual.rst
999–1000 ↗(On Diff #213212)

Awesome, thank you!

This revision is now accepted and ready to land.Aug 5 2019, 9:50 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2019, 9:52 AM
This comment was removed by xbolva00.