This is an archive of the discontinued LLVM Phabricator instance.

[clang] Implement CFG construction for @try and @catch
ClosedPublic

Authored by thakis on Oct 21 2021, 6:12 PM.

Details

Summary

@finally is still not implemented.

With this, clang can emit -Wreturn-type warnings for functions containing
@try/@catch (but not yet @finally), and -Wunreachable-code also works for those
functions.

The implementation is similar to D36914.

Part of PR46693.

Diff Detail

Event Timeline

thakis requested review of this revision.Oct 21 2021, 6:12 PM
thakis created this revision.
thakis added a subscriber: arphaman.
thakis updated this revision to Diff 381450.Oct 21 2021, 6:22 PM

clang-format

hans accepted this revision.Oct 26 2021, 6:17 AM
hans added a subscriber: hans.

lgtm, just some nits.

clang/lib/Analysis/CFG.cpp
485

One "to" too much in "to either to a ...".
Should probably be *an* SEH __try (at least in my head, that S is pronounced "ess").

3888

ultra nit: period.

3895

does the fixme still stand?

3941

Why h? Could this be a range for instead? Otherwise for (i = .., e = ..., i != e) is the classic llvm style.

5790

maybe

if (const VarDecl *PD = CS->getCatchParamDecl())
  PD->print(...)
clang/test/Sema/warn-unreachable.m
1

Not important, but I think it's much more common to have %s at the end. Same for the other test file.

This revision is now accepted and ready to land.Oct 26 2021, 6:17 AM
thakis marked 4 inline comments as done.Oct 26 2021, 6:41 AM

Thanks, all done.

Looks like there's no range access to a try's catch blocks, so no for-range loop for now. Maybe I'll add range accessors for those later.

clang/lib/Analysis/CFG.cpp
3895

No, I think this is done now. Removed, thanks.

5790

oh yes, that's much nicer. thanks, done.

This revision was landed with ongoing or failed builds.Oct 26 2021, 6:45 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 26 2021, 6:45 AM

I believe there is an issue that this change introduced. It is described here. https://bugs.llvm.org/show_bug.cgi?id=52473
If anyone could provide some insight, it would be much appreciated.

I believe there is an issue that this change introduced. It is described here. https://bugs.llvm.org/show_bug.cgi?id=52473
If anyone could provide some insight, it would be much appreciated.

Thanks for the report. I think D114660 might fix this.

(I'd say that on the bug too, but the bug tracker is currently read-only.)