aaron.ballman added inline comments.
================ Comment at: clang-tidy/bugprone/BugproneTidyModule.cpp:84 + CheckFactories.registerCheck<IoFunctionsMisusedCheck>( + "bugprone-io-functions-misused"); CheckFactories.registerCheck<LambdaFunctionNameCheck>( ---------------- This name reads a bit awkwardly because usually "misused" would come before "io functions". However, given that we've already said it's prone to bugs, I think "misuse" is somewhat implied. Perhaps `bugprone-io-functions` or `bugprone-misused-io-functions'? However, I think this is actually checking an existing CERT rule and perhaps should be added to the CERT module as `cert-fio34-c` and perhaps that's the only module it should live in? ================ Comment at: clang-tidy/bugprone/IoFunctionsMisusedCheck.cpp:25-31 + has(callExpr(callee( + functionDecl(hasAnyName("::getwchar", "::getwc", "::fgetwc")) + .bind("FuncDecl")))), + has(callExpr(callee( + functionDecl(hasAnyName("::std::getwchar", "::std::getwc", + "::std::fgetwc")) + .bind("FuncDecl")))), ---------------- You can combine these `hasAnyName()` calls into one given that they are otherwise identical matchers. ================ Comment at: clang-tidy/bugprone/IoFunctionsMisusedCheck.cpp:26 + has(callExpr(callee( + functionDecl(hasAnyName("::getwchar", "::getwc", "::fgetwc")) + .bind("FuncDecl")))), ---------------- Why only the wide char versions and not the narrow char versions as well? For instance, those are problematic on systems where `sizeof(int) == sizeof(char)`. See https://wiki.sei.cmu.edu/confluence/display/c/FIO34-C.+Distinguish+between+characters+read+from+a+file+and+EOF+or+WEOF for more details. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42682 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits