subject:"\[PATCH\] D100000\: \[clang\] WIP\: Implement simpler alternative to two\-phase lookup for NRVO"

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

2021-06-29 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov updated this revision to Diff 355393.
mizvekov added a comment.

Patch now in workable state. Is Missing a few cleanups in the implementation.

Updates coroutine implicit move tests, as now they can finally match C++20 
behavior and don't need P2266  retroactively 
applied.

There are some test result changes.
These all match current GCC behaviour though: We are now broken in the same way 
as GCC is for these.
TBD what to do about these. The ambiguous overload resolution one might be easy 
to solve,
the other ones have to be studied more carefully.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaObjCXX/block-capture.mm

Index: clang/test/SemaObjCXX/block-capture.mm
===
--- clang/test/SemaObjCXX/block-capture.mm
+++ clang/test/SemaObjCXX/block-capture.mm
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx11_2b,cxx2b %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx11_2b   %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx11_2b   %s
-// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b,cxx98  %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx11_2b,cxx2b%s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx98_20,cxx11_2b %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fobjc-arc -fblocks   -verify=cxx98_2b,cxx98_20,cxx11_2b %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -fobjc-arc -fblocks -Wno-c++11-extensions -verify=cxx98_2b,cxx98_20,cxx98%s
 
 #define TEST(T) void test_##T() { \
   __block T x;\
@@ -51,31 +51,31 @@
 
 struct ConvertingRVRef {
   ConvertingRVRef();
-  ConvertingRVRef(ConvertingRVRef &) = delete; // cxx98-note {{marked deleted here}}
+  ConvertingRVRef(ConvertingRVRef &) = delete; // cxx98_20-note {{marked deleted here}}
 
   struct X {};
   ConvertingRVRef(X &&);
   operator X() const & = delete;
   operator X() &&;
 };
-TEST(ConvertingRVRef); // cxx98-error {{call to deleted constructor}}
+TEST(ConvertingRVRef); // cxx98_20-error {{call to deleted constructor}}
 
 struct ConvertingCLVRef {
   ConvertingCLVRef();
   ConvertingCLVRef(ConvertingCLVRef &);
 
   struct X {};
-  ConvertingCLVRef(X &&); // cxx11_2b-note {{passing argument to parameter here}}
+  ConvertingCLVRef(X &&); // cxx2b-note {{passing argument to parameter here}}
   operator X() const &;
-  operator X() && = delete; // cxx11_2b-note {{marked deleted here}}
+  operator X() && = delete; // cxx2b-note {{marked deleted here}}
 };
-TEST(ConvertingCLVRef); // cxx11_2b-error {{invokes a deleted function}}
+TEST(ConvertingCLVRef); // cxx2b-error {{invokes a deleted function}}
 
 struct SubSubMove {};
 struct SubMove : SubSubMove {
   SubMove();
-  SubMove(SubMove &) = delete; // cxx98-note {{marked deleted here}}
+  SubMove(SubMove &) = delete; // cxx98_20-note {{marked deleted here}}
 
   SubMove(SubSubMove &&);
 };
-TEST(SubMove); // cxx98-error {{call to deleted constructor}}
+TEST(SubMove); // cxx98_20-error {{call to deleted constructor}}
Index: clang/test/SemaCXX/coroutine-rvo.cpp
===
--- clang/test/SemaCXX/coroutine-rvo.cpp
+++ clang/test/SemaCXX/coroutine-rvo.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -std=c++17 -fcoroutines-ts -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected,cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected   %s
 
 namespace std::experimental {
 template  struct coroutine_handle {
@@ -56,7 +57,9 @@
 auto final_suspend() noexcept { return suspend_never{}; }
 auto get_return_object() { return task{}; }
 static void unhandled_exception() {}
-void

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

2021-04-21 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov updated this revision to Diff 339349.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp

Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,12 +1,12 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b,cxx2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected,cxx20_2b,cxx2b-fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx20_2b,cxx11_20 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=expected,cxx11_17,cxx11_20 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=expected,cxx11_17,cxx11_20 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11_17,cxx11_20 -fcxx-exceptions -Wreturn-std-move %s
 
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK_14_17
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK_14_17
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefixes=CHECK
 
 // definitions for std::move
 namespace std {
@@ -78,9 +78,6 @@
 Base test2() {
 Derived d2;
 return d2;  // e1
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
 }
 ConstructFromDerived test3() {
   Derived d3;
@@ -89,23 +86,14 @@
 ConstructFromBase test4() {
 Derived d4;
 return d4;  // e3
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d4)"
 }
 ConvertFromDerived test5() {
 Derived d5;
 return d5;  // e4
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d5)"
 }
 ConvertFromBase test6() {
 Derived d6;
 return d6;  // e5
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d6)"
 }
 
 // These test cases should not produce the warning.
@@ -152,30 +140,18 @@
 Derived testParam1(Derived d) { return d; }
 Base testParam2(Derived d) {
 return d;  // e6
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)"
 }
 ConstructFromDerived testParam3(Derived d)

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

2021-04-20 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov updated this revision to Diff 339035.
mizvekov added a comment.
Herald added a subscriber: dexonsmith.

Initial implementation, still WIP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/P1155.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp

Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx20_2b,cxx2b -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20_2b   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=cxx11_17   -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=expected,cxx20_2b,cxx2b-fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx20_2b,cxx11_20 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify=expected,cxx11_17,cxx11_20 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify=expected,cxx11_17,cxx11_20 -fcxx-exceptions -Wreturn-std-move %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify=expected,cxx11_17,cxx11_20 -fcxx-exceptions -Wreturn-std-move %s
 
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
 // RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -Wreturn-std-move -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s -check-prefix=CHECK
@@ -78,9 +78,6 @@
 Base test2() {
 Derived d2;
 return d2;  // e1
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
 }
 ConstructFromDerived test3() {
   Derived d3;
@@ -89,23 +86,14 @@
 ConstructFromBase test4() {
 Derived d4;
 return d4;  // e3
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d4)"
 }
 ConvertFromDerived test5() {
 Derived d5;
 return d5;  // e4
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d5)"
 }
 ConvertFromBase test6() {
 Derived d6;
 return d6;  // e5
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d6)"
 }
 
 // These test cases should not produce the warning.
@@ -152,30 +140,18 @@
 Derived testParam1(Derived d) { return d; }
 Base testParam2(Derived d) {
 return d;  // e6
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)"
 }
 ConstructFromDerived testParam3(Derived d) {
   return d; // ok
 }
 ConstructFromBase testParam4(Derived d) {
 return d;  // e8
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)"
 }
 ConvertFromDerived testParam5(Derived d) {
 return d;  // e9
-// cxx11_17-warning@-1{{will be copied despite being returned by name}}
-// cxx11_17-note@-2{{to avoid copying}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"std::move(d)"
 }
 ConvertFromBase testParam6(Derived d) {
 return d;  // e10

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

2021-04-06 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov updated this revision to Diff 335689.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp

Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3046,25 +3046,22 @@
 /// \returns An aggregate which contains the Candidate and isMoveEligible
 /// and isCopyElidable methods. If Candidate is non-null, it means
 /// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(const VarDecl *VD, bool Parenthesized,
- bool ForceCXX20) {
+Sema::NRVOResult Sema::getNRVOResult(const VarDecl *VD, bool Parenthesized) {
   if (!VD)
 return NRVOResult();
 
-  bool hasCXX11 = getLangOpts().CPlusPlus11 || ForceCXX20,
-   hasCXX20 = getLangOpts().CPlusPlus20 || ForceCXX20;
   NRVOResult Res{VD, NRVOResult::MoveEligibleAndCopyElidable, Parenthesized};
 
   // (other than a function ... parameter)
   if (VD->getKind() == Decl::ParmVar) {
-downgradeNRVOResult(Res, hasCXX11);
+downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
   } else if (VD->getKind() != Decl::Var) {
 return NRVOResult();
   }
 
   // (other than ... a catch-clause parameter)
   if (VD->isExceptionVariable())
-downgradeNRVOResult(Res, hasCXX20);
+downgradeNRVOResult(Res, getLangOpts().CPlusPlus20);
 
   // ...automatic...
   if (!VD->hasLocalStorage())
@@ -3090,7 +3087,7 @@
 if (VDReferencedType.isVolatileQualified() ||
 !VDReferencedType->isObjectType())
   return NRVOResult();
-downgradeNRVOResult(Res, hasCXX20);
+downgradeNRVOResult(Res, getLangOpts().CPlusPlus20);
   } else {
 return NRVOResult();
   }
@@ -3099,7 +3096,7 @@
   // alignment cannot use NRVO.
   if (!VDType->isDependentType() && VD->hasAttr() &&
   Context.getDeclAlign(VD) > Context.getTypeAlignInChars(VDType))
-downgradeNRVOResult(Res, hasCXX11);
+downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 
   return Res;
 }
@@ -3118,7 +3115,7 @@
 /// \returns An aggregate which contains the Candidate and isMoveEligible
 /// and isCopyElidable methods. If Candidate is non-null, it means
 /// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(Expr *, bool ForceCXX2b) {
+Sema::NRVOResult Sema::getNRVOResult(Expr *) {
   if (!E)
 return NRVOResult();
   bool Parenthesized = isa(E);
@@ -3128,12 +3125,39 @@
   if (!DR || DR->refersToEnclosingVariableOrCapture())
 return NRVOResult();
   const auto *VD = dyn_cast(DR->getDecl());
-  NRVOResult Res = getNRVOResult(VD, Parenthesized, /*ForceCXX20=*/ForceCXX2b);
-  if (Res.Candidate && E->getValueKind() != VK_XValue &&
-  (ForceCXX2b || getLangOpts().CPlusPlus2b)) {
-E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
- CK_NoOp, E, nullptr, VK_XValue,
- FPOptionsOverride());
+  NRVOResult Res = getNRVOResult(VD, Parenthesized);
+  if (Res.S >= NRVOResult::MoveEligible) {
+ExprValueKind CastToKind =
+getLangOpts().CPlusPlus2b ? VK_XValue : VK_XValue;
+if (E->getValueKind() != CastToKind) {
+  E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
+   CK_NoOp, E, nullptr, CastToKind,
+   FPOptionsOverride());
+}
+  } else if (Res.Candidate &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move,
+ E->getExprLoc())) {
+QualType QT = Res.Candidate->getType();
+if (QT->isLValueReferenceType()) {
+  // Adding 'std::move' around an lvalue reference variable's name is
+  // dangerous. Don't suggest it.
+} else if (QT.getNonReferenceType()
+   .getUnqualifiedType()
+   .isTriviallyCopyableType(Context)) {
+  // Adding 'std::move' around a trivially copyable variable is probably
+  // pointless. Don't suggest it.
+} else {
+  bool IsThrow =
+  false; //(Entity.getKind() == InitializedEntity::EK_Exception);
+  SmallString<32> Str;
+  Str += "std::move(";
+  Str += Res.Candidate->getDeclName().getAsString();
+  Str += ")";
+  Diag(E->getExprLoc(), diag::warn_return_std_move)
+  << E->getSourceRange() << Res.Candidate->getDeclName() << IsThrow;
+  Diag(E->getExprLoc(), diag::note_add_std_move)
+  << FixItHint::CreateReplacement(E->getSourceRange(), Str);
+}
   }
   return Res;
 }
@@ -3185,160 +3209,6 @@
 downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 }
 
-/// Try to perform the

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

2021-04-06 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov updated this revision to Diff 335688.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D10

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp

Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3118,7 +3118,7 @@
 /// \returns An aggregate which contains the Candidate and isMoveEligible
 /// and isCopyElidable methods. If Candidate is non-null, it means
 /// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(Expr *, bool ForceCXX2b) {
+Sema::NRVOResult Sema::getNRVOResult(Expr *) {
   if (!E)
 return NRVOResult();
   bool Parenthesized = isa(E);
@@ -3128,12 +3128,38 @@
   if (!DR || DR->refersToEnclosingVariableOrCapture())
 return NRVOResult();
   const auto *VD = dyn_cast(DR->getDecl());
-  NRVOResult Res = getNRVOResult(VD, Parenthesized, /*ForceCXX20=*/ForceCXX2b);
-  if (Res.Candidate && E->getValueKind() != VK_XValue &&
-  (ForceCXX2b || getLangOpts().CPlusPlus2b)) {
-E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
- CK_NoOp, E, nullptr, VK_XValue,
- FPOptionsOverride());
+  NRVOResult Res = getNRVOResult(VD, Parenthesized);
+  if (Res.S >= NRVOResult::MoveEligible) {
+ExprValueKind CastToKind = VK_XValue;
+if (E->getValueKind() != CastToKind) {
+  E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
+   CK_NoOp, E, nullptr, CastToKind,
+   FPOptionsOverride());
+}
+  } else if (Res.Candidate &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move,
+ E->getExprLoc())) {
+QualType QT = Res.Candidate->getType();
+if (QT->isLValueReferenceType()) {
+  // Adding 'std::move' around an lvalue reference variable's name is
+  // dangerous. Don't suggest it.
+} else if (QT.getNonReferenceType()
+   .getUnqualifiedType()
+   .isTriviallyCopyableType(Context)) {
+  // Adding 'std::move' around a trivially copyable variable is probably
+  // pointless. Don't suggest it.
+} else {
+  bool IsThrow =
+  false; //(Entity.getKind() == InitializedEntity::EK_Exception);
+  SmallString<32> Str;
+  Str += "std::move(";
+  Str += Res.Candidate->getDeclName().getAsString();
+  Str += ")";
+  Diag(E->getExprLoc(), diag::warn_return_std_move)
+  << E->getSourceRange() << Res.Candidate->getDeclName() << IsThrow;
+  Diag(E->getExprLoc(), diag::note_add_std_move)
+  << FixItHint::CreateReplacement(E->getSourceRange(), Str);
+}
   }
   return Res;
 }
@@ -3185,160 +3211,6 @@
 downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 }
 
-/// Try to perform the initialization of a potentially-movable value,
-/// which is the operand to a return or throw statement.
-///
-/// This routine implements C++20 [class.copy.elision]p3, which attempts to
-/// treat returned lvalues as rvalues in certain cases (to prefer move
-/// construction), then falls back to treating them as lvalues if that failed.
-///
-/// \param ConvertingConstructorsOnly If true, follow [class.copy.elision]p3 and
-/// reject resolutions that find non-constructors, such as derived-to-base
-/// conversions or `operator T()&&` member functions. If false, do consider such
-/// conversion sequences.
-///
-/// \param Res We will fill this in if move-initialization was possible.
-/// If move-initialization is not possible, such that we must fall back to
-/// treating the operand as an lvalue, we will leave Res in its original
-/// invalid state.
-///
-/// \returns Whether we need to do the second overload resolution. If the first
-/// overload resolution fails, or if the first overload resolution succeeds but
-/// the selected constructor/operator doesn't match the additional criteria, we
-/// need to do the second overload resolution.
-static bool TryMoveInitialization(Sema , const InitializedEntity ,
-  const VarDecl *NRVOCandidate, Expr *,
-  bool ConvertingConstructorsOnly,
-  bool IsDiagnosticsCheck, ExprResult ) {
-  ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
-CK_NoOp, Value, VK_XValue, FPOptionsOverride());
-
-  Expr *InitExpr = 
-
-  InitializationKind Kind = InitializationKind::CreateCopy(
-  Value->getBeginLoc(), Value->getBeginLoc());
-
-  InitializationSequence Seq(S, Entity, Kind, InitExpr);
-
-  bool NeedSecondOverloadResolution = true;

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO WIP, not ready for review.

2021-04-06 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov created this revision.
Herald added a subscriber: lxfind.
mizvekov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D10

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp

Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3118,7 +3118,7 @@
 /// \returns An aggregate which contains the Candidate and isMoveEligible
 /// and isCopyElidable methods. If Candidate is non-null, it means
 /// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(Expr *, bool ForceCXX2b) {
+Sema::NRVOResult Sema::getNRVOResult(Expr *) {
   if (!E)
 return NRVOResult();
   bool Parenthesized = isa(E);
@@ -3128,12 +3128,38 @@
   if (!DR || DR->refersToEnclosingVariableOrCapture())
 return NRVOResult();
   const auto *VD = dyn_cast(DR->getDecl());
-  NRVOResult Res = getNRVOResult(VD, Parenthesized, /*ForceCXX20=*/ForceCXX2b);
-  if (Res.Candidate && E->getValueKind() != VK_XValue &&
-  (ForceCXX2b || getLangOpts().CPlusPlus2b)) {
-E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
- CK_NoOp, E, nullptr, VK_XValue,
- FPOptionsOverride());
+  NRVOResult Res = getNRVOResult(VD, Parenthesized);
+  if (Res.S >= NRVOResult::MoveEligible) {
+ExprValueKind CastToKind = VK_XValue;
+if (E->getValueKind() != CastToKind) {
+  E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
+   CK_NoOp, E, nullptr, CastToKind,
+   FPOptionsOverride());
+}
+  } else if (Res.Candidate &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move,
+ E->getExprLoc())) {
+QualType QT = Res.Candidate->getType();
+if (QT->isLValueReferenceType()) {
+  // Adding 'std::move' around an lvalue reference variable's name is
+  // dangerous. Don't suggest it.
+} else if (QT.getNonReferenceType()
+   .getUnqualifiedType()
+   .isTriviallyCopyableType(Context)) {
+  // Adding 'std::move' around a trivially copyable variable is probably
+  // pointless. Don't suggest it.
+} else {
+  bool IsThrow =
+  false; //(Entity.getKind() == InitializedEntity::EK_Exception);
+  SmallString<32> Str;
+  Str += "std::move(";
+  Str += Res.Candidate->getDeclName().getAsString();
+  Str += ")";
+  Diag(E->getExprLoc(), diag::warn_return_std_move)
+  << E->getSourceRange() << Res.Candidate->getDeclName() << IsThrow;
+  Diag(E->getExprLoc(), diag::note_add_std_move)
+  << FixItHint::CreateReplacement(E->getSourceRange(), Str);
+}
   }
   return Res;
 }
@@ -3185,160 +3211,6 @@
 downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 }
 
-/// Try to perform the initialization of a potentially-movable value,
-/// which is the operand to a return or throw statement.
-///
-/// This routine implements C++20 [class.copy.elision]p3, which attempts to
-/// treat returned lvalues as rvalues in certain cases (to prefer move
-/// construction), then falls back to treating them as lvalues if that failed.
-///
-/// \param ConvertingConstructorsOnly If true, follow [class.copy.elision]p3 and
-/// reject resolutions that find non-constructors, such as derived-to-base
-/// conversions or `operator T()&&` member functions. If false, do consider such
-/// conversion sequences.
-///
-/// \param Res We will fill this in if move-initialization was possible.
-/// If move-initialization is not possible, such that we must fall back to
-/// treating the operand as an lvalue, we will leave Res in its original
-/// invalid state.
-///
-/// \returns Whether we need to do the second overload resolution. If the first
-/// overload resolution fails, or if the first overload resolution succeeds but
-/// the selected constructor/operator doesn't match the additional criteria, we
-/// need to do the second overload resolution.
-static bool TryMoveInitialization(Sema , const InitializedEntity ,
-  const VarDecl *NRVOCandidate, Expr *,
-  bool ConvertingConstructorsOnly,
-  bool IsDiagnosticsCheck, ExprResult ) {
-  ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
-CK_NoOp, Value, VK_XValue, FPOptionsOverride());
-
-  Expr *InitExpr = 
-
-  InitializationKind Kind = InitializationKind::CreateCopy(
-  Value->getBeginLoc(), Value->getBeginLoc());
-
-  InitializationSequence Seq(S, Entity, Kind, InitExpr);
-
-

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO

[PATCH] D100000: [clang] WIP: Implement simpler alternative to two-phase lookup for NRVO WIP, not ready for review.

6 matches

Site Navigation

Mail list logo

Footer information