Page MenuHomePhabricator

Provide a fix-it hint for -Wswitch, which adds missing cases. If there are >3 cases, the inserted text will contain newlines so it will not be shown in console output (but will be applied by -fixit, available to libclang and clangd, etc).
Needs ReviewPublic

Authored by sammccall on Jun 10 2019, 11:21 AM.

Details

Reviewers
ilya-biryukov
Summary

Lifted the helper for spelling one scope from another from
SemaCodeComplete -> AST, to reuse it. It should be useful in other contexts too.

Diff Detail

Event Timeline

sammccall created this revision.Jun 10 2019, 11:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2019, 11:21 AM

Ping... it'd be nice to have for clangd-9, though it's getting late.

One major drawback that I see is the lack of indentation (and other format options) in the added code.
Should we have this fix at a higher level that can have formatting (either now or in the future)? E.g. in clangd directly?

lib/Sema/SemaStmt.cpp
1208

Maybe always put each case on a new line?
From my experience, having multiple cases on the same line is not very common.

I don't think we need to worry about formatting, that's the IDE's job (and whatever formatter it uses).
In general, the code that knows how to warn should also be the one that knows how to fix it, this looks like a good place to me.

Can we unblock this ? Probably this anymore though.

Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2022, 4:31 PM
Herald added a subscriber: usaxena95. · View Herald Transcript

I don't think we need to worry about formatting, that's the IDE's job (and whatever formatter it uses).
In general, the code that knows how to warn should also be the one that knows how to fix it, this looks like a good place to me.

Can we unblock this ? Probably this anymore though.

I don't think formatting concerns should block progress on this; we expect users to run clang-format to fix formatting issues instead of hoping the fix-it system gets it right.

nridge added a subscriber: nridge.Mar 19 2022, 5:12 PM

That's the general approach for clang-tidy use too, rely on clang-format for formatting the fixes.