logan-5 marked an inline comment as done.
logan-5 added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:43
+ Whitelist(
+ utils::options::parseStringList(Options.get("Whitelist", "swap"))) {}
+
----------------
JonasToth wrote:
> do you mean `std::swap`? If you it should be fully qualified.
> Doesn't `std::error_code` rely on adl, too? I think `std::cout <<` and other
> streams of the STL rely on it too, and probably many more code-constructs
> that are commonly used.
>
> That means, the list should be extended to at least all standard-library
> facilities that basically required ADL to work. And then we need data on
> different code bases (e.g. LLVM is a good start) how much noise gets
> generated.
I distinctly //don't// mean `std::swap` -- I want to whitelist any unqualified
function call spelled simply `swap`.
Overloaded operators are the poster child for ADL's usefulness, so that's why
this check has a special default-on `IgnoreOverloadedOperators` option. That
whitelists a whole ton of legitimate stuff including `std::cout << x` and
friends.
I don't see a ton of discussion online about `error_code`/`make_error_code` and
ADL being very much intertwined. I'm not particularly familiar with those
constructs myself though, and I could just be out of the loop. I do see a fair
number of unqualified calls to `make_error_code` within LLVM, though most of
those resolve to `llvm::make_error_code`, the documentation for which says it
exists because `std::make_error_code` can't be reliably/portably used with ADL.
That makes me think `make_error_code` would belong in LLVM's project-specific
whitelist configuration, not the check's default.
Speaking of which, I did run this check over LLVM while developing and found it
not particularly noisy as written. That is, it generated a fair number of
warnings, but only on constructs that, when examined closely, //were// a little
suspicious or non-obvious.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72282/new/
https://reviews.llvm.org/D72282
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
