Quuxplusone added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4812
+def warn_unqualified_call_to_std_function : Warning<
+ "unqualified call to %0">, InGroup<DiagGroup<"unqualified-std-call">>;
+
----------------
This should probably be named `-Wunqualified-std-move` resp.
`-Wunqualified-std-forward`, so that we preserve our ability to add a more
generalized `-Wunqualified-std-call` diagnostic (probably not enabled by
default) in the future.
I'd actually like to go ahead and add the generalized diagnostic right now,
since it would be about 10 extra lines of code, and at least //I// would use it
as soon as it landed in trunk! But I'm prepared for that to be controversial
and wouldn't necessarily pick that hill to die on. :) (Ah, and I'm reminded
that the generalized diagnostic would need a bigger list of hard-coded
functions to ignore, such as `swap` and `begin` and `end` and so on.)
================
Comment at: clang/test/SemaCXX/unqualified-std-call.cpp:34
+ std::move(foo);
+ move(foo); // expected-warning{{unqualified call to std::move}}
+
----------------
I'd like to see a test where the template finds `move` via ADL, rather than
being directly in the scope of a `using namespace std` directive. This is the
most important case AIUI, because we can train people not to `using namespace`
but we can't so easily train them not to accidentally forget a `std::`.
I'd like to see a couple of tests where the (function, resp. template) is
itself located inside `namespace std`. (You mention you deliberately want to
warn on this, and I think that's good, but I don't see any tests for it.)
I'd like to see a test where the `std::move` that is found by lookup is
actually `std::__1::move` with an inline namespace, because that's what libc++
actually does in practice. (This should still warn.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119670/new/
https://reviews.llvm.org/D119670
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
