JonasToth added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:43
+ Whitelist(
+ utils::options::parseStringList(Options.get("Whitelist", "swap"))) {}
+
----------------
JonasToth wrote:
> logan-5 wrote:
> > 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.
> I don't have a solid understanding of the `error_code` world as well. All I
> know is, that you specialize some templates with your own types in order to
> use the generic `error_code`-world.
> AFAIK that needs some form of ADL at some point, but that could even happen
> through the overloaded operators (`==` and `!=`), in which case that would
> already be handled. (maybe @aaron.ballman knows more?)
>
> But overloaded operators being ignored by default is good and that point is
> gone :)
Yes, `make_error_code` is used via ADL. -->
https://www.boost.org/doc/libs/1_72_0/libs/outcome/doc/html/motivation/plug_error_code.html
I think that should be in the default list for ignored functions, as it is a
standard facility.
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
