[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr marked an inline comment as done.
gribozavr added inline comments.


================
Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53
+  Finder->addMatcher(ompExecutableDirective(
+                         unless(isStandaloneDirective()),
+                         hasStructuredBlock(stmt().bind("structured-block")))
----------------
Do we need the `unless`?  `hasStructuredBlock()` just won't match standalone directives.


================
Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:65
+      Result.Nodes.getNodeAs<Stmt>("structured-block");
+  assert(StructuredBlock && "Expected to get soe OpenMP Structured Block.");
+
----------------
soe => some


================
Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:69
+      utils::ExceptionAnalyzer::State::Throwing)
+    return; // No exceptions appear to escape out of the structured block.
+
----------------
appear to => have been proven to


================
Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:72
+  // FIXME: We should provide more information about the exact location where
+  // the exception is thrown, maybe the full path the exception escapes
+
----------------
+1


================
Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:21
+
+.. option:: IgnoredExceptions
+
----------------
`IgnoredExceptionTypes`?


================
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
----------------
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?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59466/new/

https://reviews.llvm.org/D59466