Quuxplusone added subscribers: zoecarver, CaseyCarter, ldionne, EricWF,
mclow.lists.
Quuxplusone added a comment.
Re which libc++ folks could give feedback on this ADL-diagnosing patch: I don't
know precisely, but the candidates are few! @mclow.lists @ericwf @ldionne
@zoecarver @CaseyCarter.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp:61
+void templateFunction(T t) {
+ swap(t, t);
+
----------------
logan-5 wrote:
> Quuxplusone wrote:
> > This is not the idiomatic way of calling `swap`: there is no ADL swap for
> > `int`, for example (so `templateFunction<int>` will hard-error during
> > instantiation). It would probably be scope-creep to try to handle the
> > "std::swap two-step", but can you leave a TODO comment somewhere to revisit
> > this issue?
> >
> I believe this addressed by my juggling the tests around a bit.
Juggling the tests around doesn't address the fact that any code that does
`swap(a,b)` without doing `using std::swap;` first (or `begin(a)` without
`using std::begin;`) is almost certainly broken for primitive types.
My naive thought is that you would //not// do `using std::make_error_code;`
because `make_error_code` is definitely never going to be used with primitive
types. So "functions okay to call via ADL" and "functions that require the
std::swap two-step" actually are slightly different whitelists.
I was saying that although this issue is probably out-of-scope for what you're
doing in this patch, still, it would be nice to leave a TODO somewhere. ...Or
maybe you say "nah, that's so far out of scope I don't want to think about it,
and it may never get done, so even leaving a TODO is inappropriate."
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
