Clang-tidy, bugprone-exception-escape.
ClosedPublic
Actions

Authored by Ankit Meel (ankitm) on Nov 7 2020, 9:17 AM.

Details

Reviewers

Sergey Sharybin (sergey)
Sybren A. Stüvel (sybren)

Commits

rB88de58fd1af9: Clang-tidy, fix bugprone-exception-escape.

Summary

ofstream is closed by the implicit destructor. Since the current
destructors in TextStrokeRenderer and PSStrokeRenderer are doing
nothing more than that, remove them and silence the warning. And also
be safe from a destructor throwing.

For ~Depsgraph, std::function's constructor can throw. Also passing
throwing statements in the lambda will not be detected by clang-tidy.
So @Sergey Sharybin (sergey) fixed it by using lambda as template. Fixes both of these
issues.

Diff Detail

Repository

rB Blender

Branch

exception (branched from master)

Build Status

Buildable 11193
Build 11193: arc lint + arc unit

Event Timeline

Ankit Meel (ankitm) requested review of this revision.Nov 7 2020, 9:17 AM

Ankit Meel (ankitm) created this revision.

Harbormaster completed remote builds in B11171: Diff 30857.Nov 7 2020, 9:17 AM

Ankit Meel (ankitm) added a reviewer: Sergey Sharybin (sergey).Nov 7 2020, 9:19 AM

Ankit Meel (ankitm) mentioned this in T78535: Code quality day: Clang-Tidy.

To me seems fine. Wouldn't mind Sybren's second look at the depsgraph.

Although one thing which is weird, the code in function which is passed by pointer can also throw? What's the logic here?

It's weird, I cannot redo the error on web https://godbolt.org/z/MWrbqj
I'll try a release build of clang (version 11) to be sure.
If clang 12's analysis is correct, I'd guess that it's pointing out that constructor of std::function may throw std::bad_alloc or std::bad_function_call
https://en.cppreference.com/w/cpp/utility/functional/function/function

I checked with clang-11 and get same error for ~Depsgraph() locally.
And you're right that the code passed in the lambda can literally be throw 1; and clang-tidy will be okay when using function pointer.
On godbolt too, passing throw in the lambda is okay for function pointer as well as std::function. https://godbolt.org/z/Evzrch
If throw 1; is not in lambda, but in some function down the stack in the destructor, then all sirens go off.

Ankit Meel (ankitm) edited the summary of this revision. (Show Details)Nov 9 2020, 5:39 PM

Fix by Sergey: Fix ~Depsgraph by using a lambda as template parameter. Plus point is that now any throw statement in lambda can be detected by clang-tidy.

Harbormaster completed remote builds in B11193: Diff 30909.Nov 9 2020, 5:39 PM

Ankit Meel (ankitm) edited the summary of this revision. (Show Details)Nov 9 2020, 5:40 PM

Sergey Sharybin (sergey) accepted this revision.Nov 9 2020, 5:42 PM

Sybren A. Stüvel (sybren) accepted this revision.Nov 17 2020, 10:34 AM

This revision is now accepted and ready to land.Nov 17 2020, 10:34 AM

Closed by commit rB88de58fd1af9: Clang-tidy, fix bugprone-exception-escape. (authored by Ankit Meel (ankitm)). · Explain WhyNov 17 2020, 1:07 PM

This revision was automatically updated to reflect the committed changes.

Ankit Meel (ankitm) added a commit: rB88de58fd1af9: Clang-tidy, fix bugprone-exception-escape..