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

Corentin Jabot via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 24 07:09:09 PST 2022


cor3ntin added inline comments.


================
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());
----------------
erichkeane wrote:
> 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.
Oh, i see.. I guess we could not have an array but that seemed cleaner. Or we could find a less generic name for the function, if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119670



More information about the cfe-commits mailing list