[clang] 20555a1 - [clang] P2266 implicit moves STL workaround
Matheus Izvekov via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 26 13:21:40 PDT 2021
Author: Matheus Izvekov
Date: 2021-07-26T22:21:31+02:00
New Revision: 20555a15a596012ef827e29b665db53a4fc0b86c
URL: https://github.com/llvm/llvm-project/commit/20555a15a596012ef827e29b665db53a4fc0b86c
DIFF: https://github.com/llvm/llvm-project/commit/20555a15a596012ef827e29b665db53a4fc0b86c.diff
LOG: [clang] P2266 implicit moves STL workaround
This patch replaces the workaround for simpler implicit moves
implemented in D105518.
The Microsoft STL currently has some issues with P2266.
Where before, with -fms-compatibility, we would disable simpler
implicit moves globally, with this change, we disable it only
when the returned expression is in a context contained by
std namespace and is located within a system header.
Signed-off-by: Matheus Izvekov <mizvekov at gmail.com>
Reviewed By: aaron.ballman, mibintc
Differential Revision: https://reviews.llvm.org/D105951
Added:
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Frontend/InitPreprocessor.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 013722cfbe1e5..83a2d132bf6a4 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4785,20 +4785,24 @@ class Sema final {
bool isMoveEligible() const { return S != None; };
bool isCopyElidable() const { return S == MoveEligibleAndCopyElidable; }
};
- NamedReturnInfo getNamedReturnInfo(Expr *&E, bool ForceCXX2b = false);
+ enum class SimplerImplicitMoveMode { ForceOff, Normal, ForceOn };
+ NamedReturnInfo getNamedReturnInfo(
+ Expr *&E, SimplerImplicitMoveMode Mode = SimplerImplicitMoveMode::Normal);
NamedReturnInfo getNamedReturnInfo(const VarDecl *VD);
const VarDecl *getCopyElisionCandidate(NamedReturnInfo &Info,
QualType ReturnType);
- ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
- const NamedReturnInfo &NRInfo,
- Expr *Value);
+ ExprResult
+ PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
+ const NamedReturnInfo &NRInfo, Expr *Value,
+ bool SupressSimplerImplicitMoves = false);
StmtResult ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
Scope *CurScope);
StmtResult BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp);
StmtResult ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
- NamedReturnInfo &NRInfo);
+ NamedReturnInfo &NRInfo,
+ bool SupressSimplerImplicitMoves);
StmtResult ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
bool IsVolatile, unsigned NumOutputs,
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 676421552a757..bca0bb4ada672 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -598,8 +598,7 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
}
// C++2b features.
if (LangOpts.CPlusPlus2b) {
- if (!LangOpts.MSVCCompat)
- Builder.defineMacro("__cpp_implicit_move", "202011L");
+ Builder.defineMacro("__cpp_implicit_move", "202011L");
Builder.defineMacro("__cpp_size_t_suffix", "202011L");
}
if (LangOpts.Char8)
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 31a4092b5b604..098fe618865fa 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -977,7 +977,7 @@ StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E,
VarDecl *Promise = FSI->CoroutinePromise;
ExprResult PC;
if (E && (isa<InitListExpr>(E) || !E->getType()->isVoidType())) {
- getNamedReturnInfo(E, /*ForceCXX2b=*/true);
+ getNamedReturnInfo(E, SimplerImplicitMoveMode::ForceOn);
PC = buildPromiseCall(*this, Promise, Loc, "return_value", E);
} else {
E = MakeFullDiscardedValueExpr(E).get();
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 64c53d6666f0b..3baccec2d7bb4 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3322,7 +3322,8 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope *CurScope) {
/// \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::NamedReturnInfo Sema::getNamedReturnInfo(Expr *&E, bool ForceCXX2b) {
+Sema::NamedReturnInfo Sema::getNamedReturnInfo(Expr *&E,
+ SimplerImplicitMoveMode Mode) {
if (!E)
return NamedReturnInfo();
// - in a return statement in a function [where] ...
@@ -3334,13 +3335,10 @@ Sema::NamedReturnInfo Sema::getNamedReturnInfo(Expr *&E, bool ForceCXX2b) {
if (!VD)
return NamedReturnInfo();
NamedReturnInfo Res = getNamedReturnInfo(VD);
- // FIXME: We supress simpler implicit move here (unless ForceCXX2b is true)
- // in msvc compatibility mode just as a temporary work around,
- // as the MSVC STL has issues with this change.
- // We will come back later with a more targeted approach.
if (Res.Candidate && !E->isXValue() &&
- (ForceCXX2b ||
- (getLangOpts().CPlusPlus2b && !getLangOpts().MSVCCompat))) {
+ (Mode == SimplerImplicitMoveMode::ForceOn ||
+ (Mode != SimplerImplicitMoveMode::ForceOff &&
+ getLangOpts().CPlusPlus2b))) {
E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
CK_NoOp, E, nullptr, VK_XValue,
FPOptionsOverride());
@@ -3480,15 +3478,10 @@ VerifyInitializationSequenceCXX98(const Sema &S,
/// 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.
-ExprResult
-Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
- const NamedReturnInfo &NRInfo,
- Expr *Value) {
- // FIXME: We force P1825 implicit moves here in msvc compatibility mode
- // because we are disabling simpler implicit moves as a temporary
- // work around, as the MSVC STL has issues with this change.
- // We will come back later with a more targeted approach.
- if ((!getLangOpts().CPlusPlus2b || getLangOpts().MSVCCompat) &&
+ExprResult Sema::PerformMoveOrCopyInitialization(
+ const InitializedEntity &Entity, const NamedReturnInfo &NRInfo, Expr *Value,
+ bool SupressSimplerImplicitMoves) {
+ if ((!getLangOpts().CPlusPlus2b || SupressSimplerImplicitMoves) &&
NRInfo.isMoveEligible()) {
ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
CK_NoOp, Value, VK_XValue, FPOptionsOverride());
@@ -3529,7 +3522,8 @@ static bool hasDeducedReturnType(FunctionDecl *FD) {
///
StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
Expr *RetValExp,
- NamedReturnInfo &NRInfo) {
+ NamedReturnInfo &NRInfo,
+ bool SupressSimplerImplicitMoves) {
// If this is the first return we've seen, infer the return type.
// [expr.prim.lambda]p4 in C++11; block literals follow the same rules.
CapturingScopeInfo *CurCap = cast<CapturingScopeInfo>(getCurFunction());
@@ -3660,7 +3654,8 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
// the C version of which boils down to CheckSingleAssignmentConstraints.
InitializedEntity Entity = InitializedEntity::InitializeResult(
ReturnLoc, FnRetType, NRVOCandidate != nullptr);
- ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRInfo, RetValExp);
+ ExprResult Res = PerformMoveOrCopyInitialization(
+ Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
if (Res.isInvalid()) {
// FIXME: Cleanup temporaries here, anyway?
return StmtError();
@@ -3870,15 +3865,37 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
return R;
}
+static bool CheckSimplerImplicitMovesMSVCWorkaround(const Sema &S,
+ const Expr *E) {
+ if (!E || !S.getLangOpts().CPlusPlus2b || !S.getLangOpts().MSVCCompat)
+ return false;
+ const Decl *D = E->getReferencedDeclOfCallee();
+ if (!D || !S.SourceMgr.isInSystemHeader(D->getLocation()))
+ return false;
+ for (const DeclContext *DC = D->getDeclContext(); DC; DC = DC->getParent()) {
+ if (DC->isStdNamespace())
+ return true;
+ }
+ return false;
+}
+
StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
// Check for unexpanded parameter packs.
if (RetValExp && DiagnoseUnexpandedParameterPack(RetValExp))
return StmtError();
- NamedReturnInfo NRInfo = getNamedReturnInfo(RetValExp);
+ // HACK: We supress simpler implicit move here in msvc compatibility mode
+ // just as a temporary work around, as the MSVC STL has issues with
+ // this change.
+ bool SupressSimplerImplicitMoves =
+ CheckSimplerImplicitMovesMSVCWorkaround(*this, RetValExp);
+ NamedReturnInfo NRInfo = getNamedReturnInfo(
+ RetValExp, SupressSimplerImplicitMoves ? SimplerImplicitMoveMode::ForceOff
+ : SimplerImplicitMoveMode::Normal);
if (isa<CapturingScopeInfo>(getCurFunction()))
- return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp, NRInfo);
+ return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp, NRInfo,
+ SupressSimplerImplicitMoves);
QualType FnRetType;
QualType RelatedRetType;
@@ -4069,8 +4086,8 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
// we have a non-void function with an expression, continue checking
InitializedEntity Entity = InitializedEntity::InitializeResult(
ReturnLoc, RetType, NRVOCandidate != nullptr);
- ExprResult Res =
- PerformMoveOrCopyInitialization(Entity, NRInfo, RetValExp);
+ ExprResult Res = PerformMoveOrCopyInitialization(
+ Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
if (Res.isInvalid()) {
// FIXME: Clean up temporaries here anyway?
return StmtError();
diff --git a/clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp b/clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
index 07db9b8333fa6..17247c4473178 100644
--- a/clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
+++ b/clang/test/SemaCXX/cxx2b-p2266-disable-with-msvc-compat.cpp
@@ -1,52 +1,153 @@
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fcxx-exceptions -verify=new %s
-// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fcxx-exceptions -fms-compatibility -verify=old %s
-// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify=old %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify=cxx2b,new %s
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -fms-compatibility -verify=cxx2b,old %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20,old %s
// FIXME: This is a test for a temporary workaround where we disable simpler implicit moves
-// when compiling with -fms-compatibility, because the MSVC STL does not compile.
-// A better workaround is under discussion.
-// The test cases here are just a copy from `CXX/class/class.init/class.copy.elision/p3.cpp`,
-// so feel free to delete this file when the workaround is not needed anymore.
-
-struct CopyOnly {
- CopyOnly(); // new-note {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
- // new-note at -1 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
- CopyOnly(CopyOnly &); // new-note {{candidate constructor not viable: expects an lvalue for 1st argument}}
- // new-note at -1 {{candidate constructor not viable: expects an lvalue for 1st argument}}
-};
-struct MoveOnly {
- MoveOnly();
- MoveOnly(MoveOnly &&);
+// in the STL when compiling with -fms-compatibility, because of issues with the
+// implementation there.
+// Feel free to delete this file when the workaround is not needed anymore.
+
+#if __INCLUDE_LEVEL__ == 0
+
+#if __cpluscplus > 202002L && __cpp_implicit_move < 202011L
+#error "__cpp_implicit_move not defined correctly"
+#endif
+
+struct nocopy {
+ nocopy(nocopy &&);
};
-MoveOnly &&rref();
-
-MoveOnly &&test1(MoveOnly &&w) {
- return w; // old-error {{cannot bind to lvalue of type}}
-}
-
-CopyOnly test2(bool b) {
- static CopyOnly w1;
- CopyOnly w2;
- if (b) {
- return w1;
- } else {
- return w2; // new-error {{no matching constructor for initialization}}
- }
-}
-
-template <class T> T &&test3(T &&x) { return x; } // old-error {{cannot bind to lvalue of type}}
-template MoveOnly &test3<MoveOnly &>(MoveOnly &);
-template MoveOnly &&test3<MoveOnly>(MoveOnly &&); // old-note {{in instantiation of function template specialization}}
-
-MoveOnly &&test4() {
- MoveOnly &&x = rref();
- return x; // old-error {{cannot bind to lvalue of type}}
-}
-
-void test5() try {
- CopyOnly x;
- throw x; // new-error {{no matching constructor for initialization}}
-} catch (...) {
-}
-
-MoveOnly test6(MoveOnly x) { return x; }
+
+int &&mt1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &mt2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+
+namespace {
+int &&mt1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &mt2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &&mt1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &mt2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+namespace std {
+int &&mt1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &mt2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace std
+} // namespace foo
+
+namespace std {
+
+int &&mt1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &mt2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+
+namespace {
+int &&mt1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &mt2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &&mt1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &mt2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy mt3(nocopy x) { return x; }
+} // namespace foo
+
+} // namespace std
+
+#include __FILE__
+
+#define SYSTEM
+#include __FILE__
+
+#elif !defined(SYSTEM)
+
+int &&ut1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &ut2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy ut3(nocopy x) { return x; }
+
+namespace {
+int &&ut1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &ut2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy ut3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &&ut1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &ut2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy ut3(nocopy x) { return x; }
+namespace std {
+int &&ut1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &ut2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy ut3(nocopy x) { return x; }
+} // namespace std
+} // namespace foo
+
+namespace std {
+
+int &&ut1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &ut2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy ut3(nocopy x) { return x; }
+
+namespace {
+int &&ut1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &ut2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy ut3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &&ut1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &ut2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy ut3(nocopy x) { return x; }
+} // namespace foo
+
+} // namespace std
+
+#else
+
+#pragma GCC system_header
+
+int &&st1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &st2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy st3(nocopy x) { return x; }
+
+namespace {
+int &&st1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &st2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy st3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &&st1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &st2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy st3(nocopy x) { return x; }
+namespace std {
+int &&st1(int &&x) { return x; } // cxx20-error {{cannot bind to lvalue}}
+int &st2(int &&x) { return x; } // cxx2b-error {{cannot bind to a temporary}}
+nocopy st3(nocopy x) { return x; }
+} // namespace std
+} // namespace foo
+
+namespace std {
+
+int &&st1(int &&x) { return x; } // old-error {{cannot bind to lvalue}}
+int &st2(int &&x) { return x; } // new-error {{cannot bind to a temporary}}
+nocopy st3(nocopy x) { return x; }
+
+namespace {
+int &&st1(int &&x) { return x; } // old-error {{cannot bind to lvalue}}
+int &st2(int &&x) { return x; } // new-error {{cannot bind to a temporary}}
+nocopy st3(nocopy x) { return x; }
+} // namespace
+
+namespace foo {
+int &&st1(int &&x) { return x; } // old-error {{cannot bind to lvalue}}
+int &st2(int &&x) { return x; } // new-error {{cannot bind to a temporary}}
+nocopy st3(nocopy x) { return x; }
+} // namespace foo
+
+} // namespace std
+
+#endif
More information about the cfe-commits
mailing list