aaron.ballman added inline comments.
================ Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:32 + llvm::StringSet<> IgnoredExceptions; + StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false); + IgnoredExceptions.insert(IgnoredExceptionsVec.begin(), ---------------- Do you need to trim any of the split strings? ================ Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:48 + return; + // Similarly, if C++ Exceptions are not enabled, nothing to do. + if (!getLangOpts().CPlusPlus || !getLangOpts().CXXExceptions) ---------------- Do you have to worry about SEH exceptions in C for this functionality? ================ 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 + ---------------- Missing a full stop at the end of the comment. ================ Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:75-76 + diag(Directive->getBeginLoc(), + "An exception thrown inside of the OpenMP '%0' region is not caught in " + "that same region.") + << getOpenMPDirectiveName(Directive->getDirectiveKind()); ---------------- Clang-tidy diagnostics are not complete sentences -- please make this horribly ungrammatical. ;-) ================ Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:9 + +As per the OpenMP specification, structured block is an executable statement, +possibly compound, with a single entry at the top and a single exit at the ---------------- , structured block -> , a structured block ================ Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:10-11 +As per the OpenMP specification, structured block is an executable statement, +possibly compound, with a single entry at the top and a single exit at the +bottom. Which means, ``throw`` may not be used to to 'exit' out of the +structured block. If an exception is not caught in the same structured block ---------------- Does this mean setjmp/longjmp out of the block is also a dangerous activity? What about gotos? ================ Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:13 +structured block. If an exception is not caught in the same structured block +it was thrown in, the behaviour is undefined / implementation defined, +the program will likely terminate. ---------------- Which is it -- undefined or implementation-defined? ================ Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:23 + + Comma separated list containing type names which are not counted as thrown + exceptions in the check. Default value is an empty string. ---------------- Comma separated -> Comma-separated 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
