lebedev.ri added inline comments.
================
Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53
+ Finder->addMatcher(ompExecutableDirective(
+ unless(isStandaloneDirective()),
+ hasStructuredBlock(stmt().bind("structured-block")))
----------------
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.
================
Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:21
+
+.. option:: IgnoredExceptions
+
----------------
gribozavr wrote:
> `IgnoredExceptionTypes`?
What do you mean?
In the check it's `Options.get("IgnoredExceptions", "")`.
That is identical to the check in bugprone.
================
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
----------------
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`.
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
