This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag
ClosedPublic

Authored by steakhal on May 20 2022, 6:59 AM.

Details

Summary

This flag was introduced by
https://github.com/llvm/llvm-project/commit/6818991d7165f68fe196922d9e5c6648dd57cc47

commit 6818991d7165f68fe196922d9e5c6648dd57cc47
Author: Ted Kremenek <kremenek@apple.com>
Date:   Mon Dec 7 22:06:12 2009 +0000

Add clang-cc option '-analyzer-opt-analyze-nested-blocks' to treat
block literals as an entry point for analyzer checks.

The last reference was removed by this commit:
https://github.com/llvm/llvm-project/commit/5c32dfc5fb1cfcff8ae3671284e17daa8da3a188

commit 5c32dfc5fb1cfcff8ae3671284e17daa8da3a188
Author: Anna Zaks <ganna@apple.com>
Date:   Fri Dec 21 01:19:15 2012 +0000

[analyzer] Add blocks and ObjC messages to the call graph.
This paves the road for constructing a better function dependency graph.
If we analyze a function before the functions it calls and inlines,
there is more opportunity for optimization.
Note, we add call edges to the called methods that correspond to
function definitions (declarations with bodies).

Consequently, we should remove this dead flag.
However, this arises a couple of burning questions.

  • Should the cc1 frontend still accept this flag - to keep tools/users passing this flag directly to cc1 (which is unsupported, unadvertised) working.
  • If we should remain backward compatible, how long?
  • How can we get rid of deprecated and obsolete flags at some point?

I'm following the same deprecation process, proposed at D126215, so we will still accept this flag with a warning.

Diff Detail

Event Timeline

steakhal created this revision.May 20 2022, 6:59 AM
steakhal requested review of this revision.May 20 2022, 6:59 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 20 2022, 6:59 AM
martong accepted this revision.May 20 2022, 7:20 AM

However, this arises a couple of burning questions.

Should the cc1 frontend still accept this flag - to keep tools/users passing this flag directly to cc1 (which is unsupported, unadvertised) working.
If we should remain backward compatible, how long?
How can we get rid of deprecated and obsolete flags at some point?

The answer might be similar to what we do in case of a checker rename (or when we move out from alpha). Such a rename have the same problems, but we have not made much efforts to combat these problem. Users had to comply.

This revision is now accepted and ready to land.May 20 2022, 7:20 AM

Uhh, ohh, don't forget to reflect this change in the ReleaseNotes.rst, so urers will be notified!

steakhal updated this revision to Diff 430978.May 20 2022, 8:06 AM

Mention this removal in the release notes.

I'm hesitating about landing this.
I think I'll mark this flag deprecated instead, to emit a warning when using this flag.
So, clang-15 would emit a warning for this, and clang-16 and above we can remove this flag completely.
WDYT @martong ?

steakhal updated this revision to Diff 432525.May 27 2022, 4:55 AM
steakhal retitled this revision from [analyzer] Drop the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag to [analyzer] Deprecate the unused 'analyzer-opt-analyze-nested-blocks' cc1 flag.
steakhal edited the summary of this revision. (Show Details)
  • Rebase on top of D126215
  • Accept the flag with a warning, rather than erroring out.

I plan to land this stack next Wednesday.

This revision was landed with ongoing or failed builds.Jun 10 2022, 4:12 AM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Jun 10 2022, 4:37 AM

One of your changes broke check-clang: http://45.33.8.238/linux/78236/step_7.txt

Please take a look and revert for now if it takes a while to fix.

(Please run tests locally before committing.)

One of your changes broke check-clang: http://45.33.8.238/linux/78236/step_7.txt

Please take a look and revert for now if it takes a while to fix.

(Please run tests locally before committing.)

Yea, sorry about the inconvenience. I always run analyzer tests.
Next time I'll also run check-clang and check-clang-tools to make sure everything works.

The 90374df15ddc58d823ca42326a76f58e748f20eb should fix this.