aaron.ballman added inline comments.

================
Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected<Attr *> getResult() {
+    if (Err)
----------------
steakhal wrote:
> If it can be used only once, we should mark this function as an `r-value` 
> function.
> There is a similar macro already defined as `LLVM_LVALUE_FUNCTION`.
> 
> Later, when you would actually call this function, you need to `std::move()` 
> the object, signifying that the original object gets destroyed in the process.
> 
> @aaron.ballman Do you think we need to define `LLVM_RVALUE_FUNCTION` or we 
> can simply use the `&&` in the function declaration?
> I've seen that you tried to substitute all `LLVM_LVALUE_FUNCTION` macros in 
> the past. What's the status on this?
The expected pattern is:
```
#if LLVM_HAS_RVALUE_REFERENCE_THIS
  void func() && {
  }
#endif
```
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchersInternal.h#L609
 (and elsewhere). However, I note that there are some unprotected uses (such as 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Tooling/Syntax/BuildTree.cpp#L437)
 and so it may be a sign that we're fine to remove 
`LLVM_HAS_RVALUE_REFERENCE_THIS` because all our supported compilers do the 
right thing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

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

Reply via email to