Page MenuHomePhabricator

[clang] Improve -Wdeclaration-after-statement
ClosedPublic

Authored by melver on Jan 13 2022, 8:48 AM.

Details

Summary

With 118f966b46cf, Clang matches GCC's behaviour and allows enabling
-Wdeclaration-after-statement with C99 and later.

However, the check for mixing declarations and code is not a constant time
algorithm, and therefore should be guarded with Diags.isIgnored().

Furthermore, improve test coverage with: non-pedantic C89 with the
warning; C11 with the warning; and when using -Wall.

Finally, mention the changed behaviour in ReleaseNotes.rst.

Diff Detail

Event Timeline

melver requested review of this revision.Jan 13 2022, 8:48 AM
melver created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2022, 8:48 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Hello,

A quick review would be much appreciated. I intend to submit this before Feb 1 so it may be included in Clang 14.

In case I have not added the appropriate reviewer(s) of the modified code, please let me know.

Thank you.

Can you also add a release note about the change?

clang/test/Sema/warn-mixed-decls-code.c
1–3 ↗(On Diff #399688)
7 ↗(On Diff #399688)
9–11 ↗(On Diff #399688)
18–21 ↗(On Diff #399688)

Looks like https://reviews.llvm.org/D114787 does the same thing and was submitted a week ago.

I'll try to integrate the additional testing you suggested and change this patch to improve the implementation to not do the checking if not required as it doesn't look entirely cheap.

Looks like https://reviews.llvm.org/D114787 does the same thing and was submitted a week ago.

I'll try to integrate the additional testing you suggested and change this patch to improve the implementation to not do the checking if not required as it doesn't look entirely cheap.

I *knew* this seemed super familiar! Sorry that my search through my email missed that. :-D Yes, please rebase on trunk and add the improved isIgnored() check. Thanks!

melver updated this revision to Diff 401653.Jan 20 2022, 8:33 AM
melver marked 4 inline comments as done.
melver retitled this revision from [clang] Respect -Wdeclaration-after-statement with C99 or later to [clang] Improve -Wdeclaration-after-statement.
melver edited the summary of this revision. (Show Details)

Rebase and rework to include improvements over already submitted code.

aaron.ballman accepted this revision.Jan 20 2022, 9:27 AM

LGTM aside from a small nit with the release notes.

clang/docs/ReleaseNotes.rst
61
This revision is now accepted and ready to land.Jan 20 2022, 9:27 AM
melver updated this revision to Diff 401689.Jan 20 2022, 10:35 AM
melver marked an inline comment as done.

Use `..` in ReleaseNotes.rst.

Thanks!

This revision was landed with ongoing or failed builds.Jan 20 2022, 10:57 AM
This revision was automatically updated to reflect the committed changes.