[PATCH] D34913: [clang-tidy] Add a new Android check "android-cloexec-socket"

[Yan Wang via Phabricator via cfe-commits]

yawanng added inline comments.


================
Comment at: clang-tidy/utils/ASTUtils.h:27
+/// <needed-flag> or <flag> | <needed-flag> | ...
+bool hasFlag(const Expr *Flags, const SourceManager &SM,
+             const LangOptions &LangOpts, const char *CloseOnExecFlag);
----------------
alexfh wrote:
> `hasFlag` is too generic and may give a totally wrong impression of what the function does. Even in the context of this utility library I would prefer to expand the name and make it less ambiguous. Something along the lines of `exprHasBitFlagWithSpelling`. I wouldn't also insist on the flag being implemented as a macro, since this restriction would prevent many valid use cases with enum or constexpr int flags.
Yes, the name is a little misleading and only consider the macro is not thorough. But for the purpose of the current Android checks, where this function is used, this simple solution can cover about 95% cases. Few use `enum` or `constexpr` for flags like `O_CLOEXEC` or other similar ones.  Moreover, if extending the function to cover the constant integer variables, it's not easy to get the value of the checked flag, which may depend on the target. Generally, in current stage, this function works well in the Android cases and it's still a TODO for other more sophisticated user cases. 


https://reviews.llvm.org/D34913