[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

Thu Feb 24 06:59:59 PST 2022

erichkeane added a comment.

I understand you still don't have commit-rights.  If you'd like, I can push this in a little bit.  Just let me know the name/email you'd like me to use.

================
Comment at: clang/lib/Sema/SemaExpr.cpp:6441
+  // Only warn for some functions deemed more frequent or problematic
+  static constexpr llvm::StringRef SpecialFunctions[] = {"move", "forward"};
+  auto it = llvm::find(SpecialFunctions, D->getName());
----------------
cor3ntin wrote:
> erichkeane wrote:
> > I find myself a touch grumpy that this is used to keep this expandable to other std functions, but still sticks itself to unary functions (particularly when the function name `DiagnosedUnqualifiedCallsToStdFunctions` isn't unary-specific).
> > 
> > I'm not sure how actionable this comment is, but I felt the need to share.  I guess I could suggest moving this to the top and making this hold a 'pair' of args + name (plus presumably a "don't check arg count" for some sort of variadic function check).
> > 
> > But, 'eh', think about it perhaps?  Maybe this comment can just exist to help out the next person who wants to add something to this function.
> To be honest, this is something I've considered, and I'll go in that direction if you ask me to.
> My thinking is that we do not have the need to do that right now, we can expand on it when we want to, and in the meantime, bailing out early on arity avoid wasting cpu cycles.
Right, its the inconsistency between the two (being extra future-proof here, and being much-less future-proof above) that bugs me.

Not enough to make you do anything here though.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119670/new/

https://reviews.llvm.org/D119670