gribozavr accepted this revision.
gribozavr marked an inline comment as done.
gribozavr added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53
+ Finder->addMatcher(ompExecutableDirective(
+ unless(isStandaloneDirective()),
+ hasStructuredBlock(stmt().bind("structured-block")))
----------------
lebedev.ri wrote:
> gribozavr wrote:
> > Do we need the `unless`? `hasStructuredBlock()` just won't match
> > standalone directives.
> *need*? no.
> But it makes zero sense semantically to look for structured block
> without first establishing that it has one.
> Sure, we now check that twice (here, and in `hasStructuredBlock()`), but that
> is up to optimizer.
>
> The fact that `hasStructuredBlock()` workarounds the assert in
> `getStructuredBlock()` is only to avoid clang-query crashes,
> it is spelled as much in the docs.
> But it makes zero sense semantically to look for structured block without
> first establishing that it has one.
IDK, in my mind it makes sense. "Does a standalone directive have a structured
block? No." is a coherent logical chain.
================
Comment at: test/clang-tidy/bugprone-exception-escape-openmp.cpp:16
+ ;
+ // FIXME: this really should be caught by bugprone-exception-escape.
+ // https://bugs.llvm.org/show_bug.cgi?id=41102
----------------
lebedev.ri wrote:
> gribozavr wrote:
> > Shouldn't the function be marked with `noexcept` for
> > `bugprone-exception-escape` to catch it? It does not know about OpenMP.
> >
> > Are you suggesting that `bugprone-exception-escape` should subsume your new
> > OpenMP-specific check?
> > Shouldn't the function be marked with noexcept for
> > bugprone-exception-escape to catch it?
>
> Right. I meant `noexcept` (or did i drop it and forgot to put it back?).
> But that changed nothing here, this is still an XFAIL.
>
> > It does not know about OpenMP.
> > Are you suggesting that bugprone-exception-escape should subsume your new
> > OpenMP-specific check?
>
> Only the `;` is the structured-block here, so `thrower()` call *should* be
> caught by `bugprone-exception-escape`.
Ah, I see.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59466/new/
https://reviews.llvm.org/D59466
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
