aaron.ballman added a comment. In D33825#2246369 <https://reviews.llvm.org/D33825#2246369>, @jfb wrote:
> MSC54-CPP refers to POF, which as I pointed out above isn't relevant anymore. > I'd much rather have a diagnostic which honors the state of things after > http://wg21.link/p0270. I agree, and I've commented on that rule to remind the folks at CERT that the rule has largely gone stale. > Additionally, lots of what MSC54-CPP intends is implementation-defined. We > shouldn't diagnose for things that clang has defined as signal safe, at least > not by default. Also agreed. ================ Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp:42 + +const char *cxxStmtText(const Stmt *stmt) { + if (llvm::isa<CXXBindTemporaryExpr>(stmt)) ---------------- I'm not super keen on this approach because as new C++ constructs are added, this will continually need to be updated, which is a maintenance burden I think we should try to avoid. I would rather use some generic wording or tie the wording automatically to something provided by the `Stmt` class. ================ Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp:44 + if (llvm::isa<CXXBindTemporaryExpr>(stmt)) + return "Binding temporary C++ expression here"; + else if (llvm::isa<CXXBoolLiteralExpr>(stmt)) ---------------- Diagnostics in clang-tidy should start with a lowercase letter ================ Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp:91 +const char *cxxDeclText(const Decl *decl) { + if (llvm::isa<CXXConstructorDecl>(decl)) + return "Constructor declared here"; ---------------- Similar concerns here. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33825/new/ https://reviews.llvm.org/D33825 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
