[PATCH] D73007: [Sema] Avoid Wrange-loop-analysis false positives

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

I think we can actually view this in more general terms, unrelated to range-based for loops.

  // X is non-trivially copyable.
  struct X { X(const X&); };
  struct Y { Y(const X&); };
  
  X retVal();
  const X& retRef();
  
  // In function scope ...
  const X &x1 = retVal(); // (1) Ok, but maybe misleading.
  const X  x2 = retRef(); // (2) Creates a copy that's unnecessary.
  const Y &y1 = retVal(); // (3) Ok, but maybe misleading, and maybe not intended.
  const Y &y2 = retRef(); // (4) Ok, but maybe misleading, and maybe not intended.

My point is that the standard explicitly allows binding temporaries to `const` references and mandates a lifetime extension for the temporary that makes this safe.

It definitely makes sense to warn about (2) in `-Wall` under certain circumstances, because it might be a performance issue. (We have the similar `-Wpessimizing-move`.) But (1) is a stylistic issue, and doesn't belong in `-Wall`. I'm somewhat uncertain about (3) and (4): these are implicit conversions at work and one could argue that we're not warning about them anywhere else, but one could also argue that this is a bit harder to follow in a range-based for loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73007