Quuxplusone added inline comments.

================
Comment at: clang/lib/AST/ASTImporter.cpp:1152
+  if (Error E = importSeq(ToElementType, ToSizeExpr))
+    return std::move(E);
 
----------------
martong wrote:
> Quuxplusone wrote:
> > As the author of [P1155 "More Implicit Move"](https://wg21.link/p1155), I 
> > would expect that you don't need `return std::move(E)` — `return E` should 
> > just as well perform "implicit move" in C++11 and later, assuming that 
> > `llvm::Expected<QualType>` has a valid constructor from `llvm::Error&&`.
> > 
> > You're not seeing any compiler diagnostic that //suggests// you use 
> > `std::move` here, are you?
> I have some vague and obscure memory about that GCC 4.8 (or 5.2) required 
> explicitly the && cast, otherwise we had a diagnostic (@gamesh411 maybe you 
> help me to recall that?)
Ah, you are absolutely correct: GCC 4.9.4 does not do implicit move in this 
situation, whereas GCC 5.1 and later do do it.
https://godbolt.org/z/YDHtTH
I didn't realize that portability back to GCC 4.9 was still a concern for 
LLVM/Clang. If it is, then please feel free to keep the `std::move` here and 
throughout (but maybe mention it in the commit message for the benefit of 
others who might think as I did).  If portability back to pre-GCC-5 is //not// 
a concern, then I would continue to recommend removing the `std::move`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73675



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to