[clang] [clang] Add `clang::behaves_like_std(...)` attribute (PR #76596)
Max Winkler via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 29 16:18:03 PST 2023
https://github.com/MaxEW707 created https://github.com/llvm/llvm-project/pull/76596
#### Background ####
https://godbolt.org/z/hv53svTrq for reference on all of the below.
In games debug performance is critical as much as optimized performance.
We mainly accomplish this by reducing the amount of function calls being made in debug builds. This does not imply the use of `always_inline` but mainly being diligent in the amount of small functions being called such as getters and accessor functions.
As has been gaining support the last couple years and shown by clangs builtin support there are a lot of language level features that are implemented in the standard library such as `std::move` which lead to poor debugging performance, experience and extra compile time due to template instantiations.
Since clang already treats these meta cast functions as implicit builtins I will assume the reasoning for such is already a given.
However us and many other game studios do not use the `std` and have our own core libraries and/or STL implementations. Therefore we do not get the benefits that clang provides by recognizing certain functions inside the `std` namespace. I will try to list some reasons why we cannot just include `<utility>` and use the std functions. I am happy to expand on those reasons in the comments if desired. The EASTL paper [here](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2271.html) is a good starting point and still relevant today in my opinion.
#### Use Case ####
We have our own core libraries and STL implementation.
Internally our STL and core libraries use `static_cast` to implement move and forward semantics.
However almost all user code, the engine and games themselves, use the provided move and forward template function from these libraries.
Currently we have two variants due to historical reasons that will most likely never converge. One is inside a namespace like so `MySTL::move(...)` and one is prefixed as is done in C code like so `MyMove(...)`.
#### ClangCL ####
Last year MSVC added `[[msvc::intrinsic]]` for us game devs [here](https://github.com/MicrosoftDocs/cpp-docs/blob/main/docs/cpp/attributes.md#msvcintrinsic). This was explicitly added as an attribute under the request of us since a lot of us have custom STLs.
Clang currently lacks such an attribute which means we can't fully move onto `ClangCL` until we have a similar facility. It would also be helpful to have this attribute for our other platforms which are mostly console vendors who supply their own fork of clang but more on that below.
Besides the overloaded use of the word `intrinsic` one downside to this approach is that any warnings that could be generated by knowledge of `std` function implicit behaviours are not generated. Pessimizing move return warnings aren't generated.
This is still the case for our explicit core code that uses `static_cast` and something I plan to improve within Clang in the future.
#### Force Inline Move ####
To get around the GCC/Clang we do currently mark our move/forward functions as force inline which is respected in debug.
However as the godbolt shows the codegen isn't as good as the builtin in debug.
We still incur the compile-time cost compared to the builtin.
#### Just use std ####
While we might be able to use `std::move` via using declarations and macros that expand to `std::move` we cannot at all suffer the include times from vendor std implementations. Do note that we need to ship across different STL versions and vendor STLs that are not one of the major 3 public STLs that we all know.
MSVC STL shipped with MSVC 1938 takes 45ms to parse with clang.
libstdc++ shipped with Ubuntu 22.04 takes 31ms to parse with clang.
libc++ shipped with Ubuntu 22.04 takes 156ms to parse with clang.
We cannot include such costly headers into basically every single source file. We also don't want to balloon our PCH headers with expensive std includes and instead keep them for our internal includes as much as possible since the larger the PCH the slower your builds.
I haven't looked into `-fpch-instantiate-templates` yet or whether MSVC `/Yc` still has the same behaviour as `-fpch-instantiate-templates`. As MSVC gets more conforming and since we have to deal with vendor clang forks and vendor libraries, some which include templates in their headers, I don't want to be enforcing non-conforming defaults on our game teams using our core libraries.
#### Just declare move ####
Clang only looks for the declaration so we can just declare `move` inside the `std` namespace.
We have done this in the past however `libc++` started marking a bunch of functions with `abi_tag` which cannot be redeclared as shown in the godbolt.
We still need to ensure that our headers work with `std` headers included after us because there are some platform specific source files that need to interact with vendor or third party libraries who do use and include `std` from their headers.
#### `clang::behaves_like_std` ####
Introducing `behaves_like_std` attribute which can be used to mark a function that has the same guarantees as the `std` equivalent.
`behaves_like_std("move")` will notify clang that your function has the same semantics as `std::move` and should be treated as such.
This also has the benefit of still emitting all the `std` warnings such as self move and pessimizing move.
In the future we can probably extend this attribute to support more `std` behaviours especially as the compiler is increasing being asked to poke into the `std` namespace in every C++ standard and as the C++ std gets more meta functions that are implemented in the library but not the language itself.
I don't mean for any of this to come off as a rant. Trying to plead my case for the viability of this attribute and its necessity for some users of C++ which in my case is the games industry.
>From ff2c4b9310950c56a4e2a2b7752f3c3c442a6805 Mon Sep 17 00:00:00 2001
From: MaxEW707 <82551778+MaxEW707 at users.noreply.github.com>
Date: Fri, 29 Dec 2023 04:02:10 -0500
Subject: [PATCH 1/2] Add `clang::behaves_like_std(...)` attribute
---
clang/include/clang/Basic/Attr.td | 9 +++
clang/include/clang/Basic/AttrDocs.td | 29 ++++++++
.../clang/Basic/DiagnosticSemaKinds.td | 2 +
clang/lib/Sema/SemaDecl.cpp | 47 +++++++++---
clang/lib/Sema/SemaDeclAttr.cpp | 16 +++++
clang/lib/Sema/SemaExpr.cpp | 21 ++++++
clang/test/CodeGenCXX/builtin-std-move.cpp | 72 +++++++++++++++++++
clang/test/CodeGenCXX/builtins.cpp | 7 ++
.../SemaCXX/err-invalid-behaves-like-std.cpp | 20 ++++++
.../SemaCXX/unqualified-std-call-fixits.cpp | 28 +++++++-
clang/test/SemaCXX/warn-pessmizing-move.cpp | 22 ++++++
clang/test/SemaCXX/warn-redundant-move.cpp | 47 ++++++++++--
clang/test/SemaCXX/warn-self-move.cpp | 55 ++++++++++++++
13 files changed, 359 insertions(+), 16 deletions(-)
create mode 100644 clang/test/SemaCXX/err-invalid-behaves-like-std.cpp
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index db17211747b17d..5ad72c7026425d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1922,6 +1922,15 @@ def Convergent : InheritableAttr {
let SimpleHandler = 1;
}
+def BehavesLikeStd : InheritableAttr {
+ let Spellings = [Clang<"behaves_like_std">];
+ let Subjects = SubjectList<[Function]>;
+ let Args = [StringArgument<"StdFunction">];
+ let LangOpts = [CPlusPlus];
+ let PragmaAttributeSupport = 0;
+ let Documentation = [BehavesLikeStdDocs];
+}
+
def NoInline : DeclOrStmtAttr {
let Spellings = [CustomKeyword<"__noinline__">, GCC<"noinline">,
CXX11<"clang", "noinline">, C23<"clang", "noinline">,
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 98a7ecc7fd7df3..5d99bb87587ceb 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -577,6 +577,35 @@ effect applies only to a specific function pointer. For example:
}];
}
+def BehavesLikeStdDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+This function attribute can be used to tag functions that behave like `std` functions.
+This allows custom STL libraries in non-freestanding environments to get the same benefits
+as the `std` functions that are treated like builtins without conflicting with the `std` declarations
+and without including costly `std` headers.
+
+This attribute currently supports all `std` functions that are implicitly treated as builtins which include
+`std::addressof`, `std::forward`, `std::forward_like`, `std::move`, `std::move_if_noexcept`, and `std::as_const`.
+
+.. code-block:: c
+
+ namespace MySTL {
+ template<class T>
+ [[clang::behaves_like_std("move")]] constexpr remove_reference_t<T>&& move(T &&t) noexcept;
+ }
+
+ template<class T>
+ [[clang::behaves_like_std("move")]] constexpr remove_reference_t<T>&& myMove(T &&t) noexcept;
+
+ void example(std::string a, std::string b) {
+ foo(MySTL::move(a));
+ foo(myMove(b));
+ }
+
+ }];
+}
+
def NoInlineDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index aebb7d9b945c33..4ebcda089ca307 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3132,6 +3132,8 @@ def err_attribute_no_member_function : Error<
def err_attribute_parameter_types : Error<
"%0 attribute parameter types do not match: parameter %1 of function %2 has type %3, "
"but parameter %4 of function %5 has type %6">;
+def err_attribute_invalid_std_builtin : Error<
+ "not a valid std builtin for attribute %0">;
def err_attribute_too_many_arguments : Error<
"%0 attribute takes no more than %1 argument%s1">;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index ffbe317d559995..2498af5c7e6a1a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9753,14 +9753,11 @@ static Scope *getTagInjectionScope(Scope *S, const LangOptions &LangOpts) {
return S;
}
-/// Determine whether a declaration matches a known function in namespace std.
-static bool isStdBuiltin(ASTContext &Ctx, FunctionDecl *FD,
- unsigned BuiltinID) {
+/// Determine whether a declaration matches a known cast function in namespace
+/// std.
+static bool isStdCastBuiltin(ASTContext &Ctx, FunctionDecl *FD,
+ unsigned BuiltinID) {
switch (BuiltinID) {
- case Builtin::BI__GetExceptionInfo:
- // No type checking whatsoever.
- return Ctx.getTargetInfo().getCXXABI().isMicrosoft();
-
case Builtin::BIaddressof:
case Builtin::BI__addressof:
case Builtin::BIforward:
@@ -9774,12 +9771,23 @@ static bool isStdBuiltin(ASTContext &Ctx, FunctionDecl *FD,
const auto *FPT = FD->getType()->castAs<FunctionProtoType>();
return FPT->getNumParams() == 1 && !FPT->isVariadic();
}
-
default:
return false;
}
}
+/// Determine whether a declaration matches a known function in namespace std.
+static bool isStdBuiltin(ASTContext &Ctx, FunctionDecl *FD,
+ unsigned BuiltinID) {
+ switch (BuiltinID) {
+ case Builtin::BI__GetExceptionInfo:
+ // No type checking whatsoever.
+ return Ctx.getTargetInfo().getCXXABI().isMicrosoft();
+ default:
+ return isStdCastBuiltin(Ctx, FD, BuiltinID);
+ }
+}
+
NamedDecl*
Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
TypeSourceInfo *TInfo, LookupResult &Previous,
@@ -10704,11 +10712,28 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
// If this is the first declaration of a library builtin function, add
// attributes as appropriate.
if (!D.isRedeclaration()) {
- if (IdentifierInfo *II = Previous.getLookupName().getAsIdentifierInfo()) {
+ IdentifierInfo *II = nullptr;
+ if (auto *GA = NewFD->getAttr<BehavesLikeStdAttr>()) {
+ auto iter = PP.getIdentifierTable().find(GA->getStdFunction());
+ if (iter != PP.getIdentifierTable().end())
+ II = iter->getValue();
+ else
+ Diag(NewFD->getLocation(), diag::err_attribute_invalid_std_builtin)
+ << GA;
+ } else {
+ II = Previous.getLookupName().getAsIdentifierInfo();
+ }
+ if (II) {
if (unsigned BuiltinID = II->getBuiltinID()) {
bool InStdNamespace = Context.BuiltinInfo.isInStdNamespace(BuiltinID);
- if (!InStdNamespace &&
- NewFD->getDeclContext()->getRedeclContext()->isFileContext()) {
+ if (NewFD->hasAttr<BehavesLikeStdAttr>()) {
+ if (!InStdNamespace || !isStdCastBuiltin(Context, NewFD, BuiltinID))
+ Diag(NewFD->getLocation(), diag::err_attribute_invalid_std_builtin)
+ << NewFD->getAttr<BehavesLikeStdAttr>();
+ NewFD->addAttr(BuiltinAttr::Create(Context, BuiltinID));
+ } else if (!InStdNamespace && NewFD->getDeclContext()
+ ->getRedeclContext()
+ ->isFileContext()) {
if (NewFD->getLanguageLinkage() == CLanguageLinkage) {
// Validate the type matches unless this builtin is specified as
// matching regardless of its declared type.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index af8b90ecfed973..a48e0c19fce23a 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8485,6 +8485,19 @@ static void handleNoUniqueAddressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(NoUniqueAddressAttr::Create(S.Context, AL));
}
+static void handleBehavesLikeStdAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (AL.getNumArgs() > 1) {
+ S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
+ return;
+ }
+
+ StringRef Str;
+ if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
+ return;
+
+ D->addAttr(BehavesLikeStdAttr::Create(S.Context, Str, AL));
+}
+
static void handleSYCLKernelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
// The 'sycl_kernel' attribute applies only to function templates.
const auto *FD = cast<FunctionDecl>(D);
@@ -9397,6 +9410,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_NoUniqueAddress:
handleNoUniqueAddressAttr(S, D, AL);
break;
+ case ParsedAttr::AT_BehavesLikeStd:
+ handleBehavesLikeStdAttr(S, D, AL);
+ break;
case ParsedAttr::AT_AvailableOnlyInDefaultEvalMethod:
handleAvailableOnlyInDefaultEvalMethod(S, D, AL);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 960f513d1111b2..4adac2e61fdc52 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7145,6 +7145,27 @@ static void DiagnosedUnqualifiedCallsToStdFunctions(Sema &S,
if (BuiltinID != Builtin::BImove && BuiltinID != Builtin::BIforward)
return;
+ if (auto *GA = FD->getAttr<BehavesLikeStdAttr>()) {
+ if (auto *DC = FD->getDeclContext()) {
+ const NamespaceDecl *NSD = nullptr;
+ while (DC->isNamespace()) {
+ NSD = cast<NamespaceDecl>(DC);
+ if (NSD->isInline())
+ DC = NSD->getParent();
+ else
+ break;
+ }
+ if (NSD && NSD->getIdentifier()) {
+ std::string Name = NSD->getIdentifier()->getName().str() + "::";
+ S.Diag(DRE->getLocation(),
+ diag::warn_unqualified_call_to_std_cast_function)
+ << FD->getQualifiedNameAsString()
+ << FixItHint::CreateInsertion(DRE->getLocation(), Name);
+ }
+ }
+ return;
+ }
+
S.Diag(DRE->getLocation(), diag::warn_unqualified_call_to_std_cast_function)
<< FD->getQualifiedNameAsString()
<< FixItHint::CreateInsertion(DRE->getLocation(), "std::");
diff --git a/clang/test/CodeGenCXX/builtin-std-move.cpp b/clang/test/CodeGenCXX/builtin-std-move.cpp
index 6eb7073d67f1cc..91f9c3d06f04c6 100644
--- a/clang/test/CodeGenCXX/builtin-std-move.cpp
+++ b/clang/test/CodeGenCXX/builtin-std-move.cpp
@@ -11,6 +11,20 @@ namespace std {
template<typename T, typename U> T move(U source, U source_end, T dest);
}
+namespace mystd {
+ template<typename T> [[clang::behaves_like_std("move")]] constexpr T &&move(T &val) { return static_cast<T&&>(val); }
+ template<typename T> [[clang::behaves_like_std("move_if_noexcept")]] constexpr T &&move_if_noexcept(T &val);
+ template<typename T> [[clang::behaves_like_std("forward")]] constexpr T &&forward(T &val);
+ template<typename U, typename T> [[clang::behaves_like_std("forward_like")]] constexpr T &&forward_like(T &&val);
+ template<typename T> [[clang::behaves_like_std("as_const")]] constexpr const T &as_const(T &val);
+}
+
+template<typename T> [[clang::behaves_like_std("move")]] constexpr T &&mymove(T &val) { return static_cast<T&&>(val); }
+template<typename T> [[clang::behaves_like_std("move_if_noexcept")]] constexpr T &&mymove_if_noexcept(T &val);
+template<typename T> [[clang::behaves_like_std("forward")]] constexpr T &&myforward(T &val);
+template<typename U, typename T> [[clang::behaves_like_std("forward_like")]] constexpr T &&myforward_like(T &&val);
+template<typename T> [[clang::behaves_like_std("as_const")]] constexpr const T &myas_const(T &val);
+
class T {};
extern "C" void take(T &&);
extern "C" void take_lval(const T &);
@@ -27,6 +41,24 @@ T &forward_a = std::forward<T&>(a);
// CHECK-DAG: @forward_like_a = constant ptr @a
T &forward_like_a = std::forward_like<int&>(a);
+// CHECK-DAG: @move_a_2 = constant ptr @a
+T &&move_a_2 = mystd::move(a);
+// CHECK-DAG: @move_if_noexcept_a_2 = constant ptr @a
+T &&move_if_noexcept_a_2 = mystd::move_if_noexcept(a);
+// CHECK-DAG: @forward_a_2 = constant ptr @a
+T &forward_a_2 = mystd::forward<T&>(a);
+// CHECK-DAG: @forward_like_a_2 = constant ptr @a
+T &forward_like_a_2 = mystd::forward_like<int&>(a);
+
+// CHECK-DAG: @move_a_3 = constant ptr @a
+T &&move_a_3 = mymove(a);
+// CHECK-DAG: @move_if_noexcept_a_3 = constant ptr @a
+T &&move_if_noexcept_a_3 = mymove_if_noexcept(a);
+// CHECK-DAG: @forward_a_3 = constant ptr @a
+T &forward_a_3 = myforward<T&>(a);
+// CHECK-DAG: @forward_like_a_3 = constant ptr @a
+T &forward_like_a_3 = myforward_like<int&>(a);
+
// Check emission of a non-constant call.
// CHECK-LABEL: define {{.*}} void @test
extern "C" void test(T &t) {
@@ -53,6 +85,46 @@ extern "C" void test(T &t) {
// CHECK: declare {{.*}} @_ZSt4moveI1TS0_ET_T0_S2_S1_
+// CHECK-LABEL: define {{.*}} void @test2
+extern "C" void test2(T &t) {
+ // CHECK: store ptr %{{.*}}, ptr %[[T_REF:[^,]*]]
+ // CHECK: %0 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take(ptr {{.*}} %0)
+ take(mystd::move(t));
+ // CHECK: %1 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take(ptr {{.*}} %1)
+ take(mystd::move_if_noexcept(t));
+ // CHECK: %2 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take(ptr {{.*}} %2)
+ take(mystd::forward<T&&>(t));
+ // CHECK: %3 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take_lval(ptr {{.*}} %3)
+ take_lval(mystd::forward_like<int&>(t));
+ // CHECK: %4 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take_lval(ptr {{.*}} %4)
+ take_lval(mystd::as_const<T&&>(t));
+}
+
+// CHECK-LABEL: define {{.*}} void @test3
+extern "C" void test3(T &t) {
+ // CHECK: store ptr %{{.*}}, ptr %[[T_REF:[^,]*]]
+ // CHECK: %0 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take(ptr {{.*}} %0)
+ take(mymove(t));
+ // CHECK: %1 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take(ptr {{.*}} %1)
+ take(mymove_if_noexcept(t));
+ // CHECK: %2 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take(ptr {{.*}} %2)
+ take(myforward<T&&>(t));
+ // CHECK: %3 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take_lval(ptr {{.*}} %3)
+ take_lval(myforward_like<int&>(t));
+ // CHECK: %4 = load ptr, ptr %[[T_REF]]
+ // CHECK: call void @take_lval(ptr {{.*}} %4)
+ take_lval(myas_const<T&&>(t));
+}
+
// Check that we instantiate and emit if the address is taken.
// CHECK-LABEL: define {{.*}} @use_address
extern "C" void *use_address() {
diff --git a/clang/test/CodeGenCXX/builtins.cpp b/clang/test/CodeGenCXX/builtins.cpp
index 90265186fb3d8c..aea494dcb5a942 100644
--- a/clang/test/CodeGenCXX/builtins.cpp
+++ b/clang/test/CodeGenCXX/builtins.cpp
@@ -31,6 +31,7 @@ S *addressof(bool b, S &s, S &t) {
}
namespace std { template<typename T> T *addressof(T &); }
+namespace mystd { template<typename T> [[clang::behaves_like_std("addressof")]] T *addressof(T &); }
// CHECK: define {{.*}} @_Z13std_addressofbR1SS0_(
S *std_addressof(bool b, S &s, S &t) {
@@ -39,6 +40,12 @@ S *std_addressof(bool b, S &s, S &t) {
return std::addressof(b ? s : t);
}
+S *mystd_addressof(bool b, S &s, S &t) {
+ // CHECK: %[[LVALUE:.*]] = phi
+ // CHECK: ret ptr %[[LVALUE]]
+ return mystd::addressof(b ? s : t);
+}
+
namespace std { template<typename T> T *__addressof(T &); }
// CHECK: define {{.*}} @_Z15std___addressofbR1SS0_(
diff --git a/clang/test/SemaCXX/err-invalid-behaves-like-std.cpp b/clang/test/SemaCXX/err-invalid-behaves-like-std.cpp
new file mode 100644
index 00000000000000..9374ae9461b045
--- /dev/null
+++ b/clang/test/SemaCXX/err-invalid-behaves-like-std.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+
+namespace mystd {
+inline namespace bar {
+template <class T> struct remove_reference { typedef T type; };
+template <class T> struct remove_reference<T&> { typedef T type; };
+template <class T> struct remove_reference<T&&> { typedef T type; };
+
+template <class T>
+[[clang::behaves_like_std("moved")]] typename remove_reference<T>::type &&move(T &&t); // expected-error {{not a valid std builtin for attribute 'behaves_like_std'}}
+
+template <class T>
+[[clang::behaves_like_std("__builtin_abs")]] typename remove_reference<T>::type &&move2(T &&t); // expected-error {{not a valid std builtin for attribute 'behaves_like_std'}}
+
+template <class T>
+[[clang::behaves_like_std("strlen")]] typename remove_reference<T>::type &&move3(T &&t); // expected-error {{not a valid std builtin for attribute 'behaves_like_std'}}
+
+}
+}
+
diff --git a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp b/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
index 1a1ffc7ba2e822..7e48b40ac346fd 100644
--- a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
+++ b/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
@@ -12,12 +12,38 @@ int &&forward(auto &a) { return a; }
} // namespace std
-using namespace std;
+namespace mystd {
+
+[[clang::behaves_like_std("move")]] int &&move(auto &&a) { return a; }
+
+[[clang::behaves_like_std("forward")]] int &&forward(auto &a) { return a; }
+
+} // namespace mystd
+
+[[clang::behaves_like_std("move")]] int &&mymove(auto &&a) { return a; }
+
+[[clang::behaves_like_std("forward")]] int &&myforward(auto &a) { return a; }
void f() {
+ using namespace std;
int i = 0;
(void)move(i); // expected-warning {{unqualified call to 'std::move}}
// CHECK: {{^}} (void)std::move
(void)forward(i); // expected-warning {{unqualified call to 'std::forward}}
// CHECK: {{^}} (void)std::forward
}
+
+void g() {
+ using namespace mystd;
+ int i = 0;
+ (void)move(i); // expected-warning {{unqualified call to 'mystd::move}}
+ // CHECK: {{^}} (void)mystd::move
+ (void)forward(i); // expected-warning {{unqualified call to 'mystd::forward}}
+ // CHECK: {{^}} (void)mystd::forward
+}
+
+void h() {
+ int i = 0;
+ (void)mymove(i); // no-warning
+ (void)myforward(i); // no-warning
+}
diff --git a/clang/test/SemaCXX/warn-pessmizing-move.cpp b/clang/test/SemaCXX/warn-pessmizing-move.cpp
index 2c27cd7f95f74d..d9c63dda10d1cd 100644
--- a/clang/test/SemaCXX/warn-pessmizing-move.cpp
+++ b/clang/test/SemaCXX/warn-pessmizing-move.cpp
@@ -13,6 +13,16 @@ template <class T> typename remove_reference<T>::type &&move(T &&t);
}
}
+namespace mystd {
+inline namespace bar {
+template <class T> struct remove_reference { typedef T type; };
+template <class T> struct remove_reference<T&> { typedef T type; };
+template <class T> struct remove_reference<T&&> { typedef T type; };
+
+template <class T> [[clang::behaves_like_std("move")]] typename remove_reference<T>::type &&move(T &&t);
+}
+}
+
struct A {
#ifdef USER_DEFINED
A() {}
@@ -39,6 +49,18 @@ A test1(A a1) {
// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:22-[[@LINE-4]]:23}:""
}
+A test1_mystd(A a1) {
+ A a2;
+ return a1;
+ return a2;
+ return mystd::move(a1);
+ return mystd::move(a2);
+ // expected-warning at -1{{prevents copy elision}}
+ // expected-note at -2{{remove std::move call}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:22}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:25}:""
+}
+
B test2(A a1, B b1) {
// Object is different than return type so don't warn.
A a2;
diff --git a/clang/test/SemaCXX/warn-redundant-move.cpp b/clang/test/SemaCXX/warn-redundant-move.cpp
index 2bfc8c9312f02e..0617c06702fcd4 100644
--- a/clang/test/SemaCXX/warn-redundant-move.cpp
+++ b/clang/test/SemaCXX/warn-redundant-move.cpp
@@ -13,6 +13,16 @@ template <class T> typename remove_reference<T>::type &&move(T &&t);
}
}
+namespace mystd {
+inline namespace baz {
+template <class T> struct remove_reference { typedef T type; };
+template <class T> struct remove_reference<T&> { typedef T type; };
+template <class T> struct remove_reference<T&&> { typedef T type; };
+
+template <class T> [[clang::behaves_like_std("move")]] typename remove_reference<T>::type &&move(T &&t);
+}
+}
+
// test1 and test2 should not warn until after implementation of DR1579.
struct A {};
struct B : public A {};
@@ -86,6 +96,21 @@ D test5(D d) {
// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:22}:""
}
+D test6(D d) {
+ return d;
+ // Verify the implicit move from the AST dump
+ // CHECK-AST: ReturnStmt{{.*}}line:[[@LINE-2]]
+ // CHECK-AST-NEXT: CXXConstructExpr{{.*}}D{{.*}}void (D &&)
+ // CHECK-AST-NEXT: ImplicitCastExpr
+ // CHECK-AST-NEXT: DeclRefExpr{{.*}}ParmVar{{.*}}'d'
+
+ return mystd::move(d);
+ // expected-warning at -1{{redundant move in return statement}}
+ // expected-note at -2{{remove std::move call here}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:22}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:23-[[@LINE-4]]:24}:""
+}
+
namespace templates {
struct A {};
struct B { B(A); };
@@ -104,13 +129,27 @@ namespace templates {
test1<B>(A());
}
+ // Warn once here since the type is not dependent.
+ template <typename T>
+ A test2(A a) {
+ return mystd::move(a);
+ // expected-warning at -1{{redundant move in return statement}}
+ // expected-note at -2{{remove std::move call here}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:24}:""
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:25-[[@LINE-4]]:26}:""
+ }
+ void run_test2() {
+ test2<A>(A());
+ test2<B>(A());
+ }
+
// T1 and T2 may not be the same, the warning may not always apply.
template <typename T1, typename T2>
- T1 test2(T2 t) {
+ T1 test3(T2 t) {
return std::move(t);
}
- void run_test2() {
- test2<A, A>(A());
- test2<B, A>(A());
+ void run_test3() {
+ test3<A, A>(A());
+ test3<B, A>(A());
}
}
diff --git a/clang/test/SemaCXX/warn-self-move.cpp b/clang/test/SemaCXX/warn-self-move.cpp
index 0987e9b6bf6017..3407820b351bf2 100644
--- a/clang/test/SemaCXX/warn-self-move.cpp
+++ b/clang/test/SemaCXX/warn-self-move.cpp
@@ -11,6 +11,17 @@ template <class T> typename remove_reference<T>::type &&move(T &&t);
}
}
+namespace mystd {
+inline namespace bar {
+template <class T> struct remove_reference { typedef T type; };
+template <class T> struct remove_reference<T&> { typedef T type; };
+template <class T> struct remove_reference<T&&> { typedef T type; };
+
+template <class T>
+[[clang::behaves_like_std("move")]] typename remove_reference<T>::type &&move(T &&t);
+}
+}
+
void int_test() {
int x = 5;
x = std::move(x); // expected-warning{{explicitly moving}}
@@ -21,6 +32,16 @@ void int_test() {
expected-warning {{unqualified call to 'std::move}}
}
+void int_test_mystd() {
+ int x = 5;
+ x = mystd::move(x); // expected-warning{{explicitly moving}}
+ (x) = mystd::move(x); // expected-warning{{explicitly moving}}
+
+ using mystd::move;
+ x = move(x); // expected-warning{{explicitly moving}} \
+ expected-warning {{unqualified call to 'mystd::move}}
+}
+
int global;
void global_int_test() {
global = std::move(global); // expected-warning{{explicitly moving}}
@@ -31,6 +52,15 @@ void global_int_test() {
expected-warning {{unqualified call to 'std::move}}
}
+void global_int_test_mystd() {
+ global = mystd::move(global); // expected-warning{{explicitly moving}}
+ (global) = mystd::move(global); // expected-warning{{explicitly moving}}
+
+ using mystd::move;
+ global = move(global); // expected-warning{{explicitly moving}} \
+ expected-warning {{unqualified call to 'mystd::move}}
+}
+
class field_test {
int x;
field_test(field_test&& other) {
@@ -44,6 +74,19 @@ class field_test {
}
};
+class field_test_mystd {
+ int x;
+ field_test_mystd(field_test_mystd&& other) {
+ x = mystd::move(x); // expected-warning{{explicitly moving}}
+ x = mystd::move(other.x);
+ other.x = mystd::move(x);
+ other.x = mystd::move(other.x); // expected-warning{{explicitly moving}}
+ }
+ void withSuggest(int x) {
+ x = mystd::move(x); // expected-warning{{explicitly moving variable of type 'int' to itself; did you mean to move to member 'x'?}}
+ }
+};
+
struct A {};
struct B { A a; };
struct C { C() {}; ~C() {} };
@@ -58,3 +101,15 @@ void struct_test() {
C c;
c = std::move(c); // expected-warning{{explicitly moving}}
}
+
+void struct_test_mystd() {
+ A a;
+ a = mystd::move(a); // expected-warning{{explicitly moving}}
+
+ B b;
+ b = mystd::move(b); // expected-warning{{explicitly moving}}
+ b.a = mystd::move(b.a); // expected-warning{{explicitly moving}}
+
+ C c;
+ c = mystd::move(c); // expected-warning{{explicitly moving}}
+}
>From 638af52d8ef11b37e6c4d1e072793575fe579013 Mon Sep 17 00:00:00 2001
From: MaxEW707 <82551778+MaxEW707 at users.noreply.github.com>
Date: Fri, 29 Dec 2023 17:58:20 -0500
Subject: [PATCH 2/2] formatting fixes
---
.../SemaCXX/unqualified-std-call-fixits.cpp | 6 +-
clang/test/SemaCXX/warn-self-move.cpp | 56 +++++++++----------
2 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp b/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
index 7e48b40ac346fd..52c3b0b6f13b6f 100644
--- a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
+++ b/clang/test/SemaCXX/unqualified-std-call-fixits.cpp
@@ -43,7 +43,7 @@ void g() {
}
void h() {
- int i = 0;
- (void)mymove(i); // no-warning
- (void)myforward(i); // no-warning
+ int i = 0;
+ (void)mymove(i); // no-warning
+ (void)myforward(i); // no-warning
}
diff --git a/clang/test/SemaCXX/warn-self-move.cpp b/clang/test/SemaCXX/warn-self-move.cpp
index 3407820b351bf2..c2344c74e6b715 100644
--- a/clang/test/SemaCXX/warn-self-move.cpp
+++ b/clang/test/SemaCXX/warn-self-move.cpp
@@ -33,13 +33,13 @@ void int_test() {
}
void int_test_mystd() {
- int x = 5;
- x = mystd::move(x); // expected-warning{{explicitly moving}}
- (x) = mystd::move(x); // expected-warning{{explicitly moving}}
+ int x = 5;
+ x = mystd::move(x); // expected-warning{{explicitly moving}}
+ (x) = mystd::move(x); // expected-warning{{explicitly moving}}
- using mystd::move;
- x = move(x); // expected-warning{{explicitly moving}} \
- expected-warning {{unqualified call to 'mystd::move}}
+ using mystd::move;
+ x = move(x); // expected-warning{{explicitly moving}} \
+ expected-warning {{unqualified call to 'mystd::move}}
}
int global;
@@ -53,12 +53,12 @@ void global_int_test() {
}
void global_int_test_mystd() {
- global = mystd::move(global); // expected-warning{{explicitly moving}}
- (global) = mystd::move(global); // expected-warning{{explicitly moving}}
+ global = mystd::move(global); // expected-warning{{explicitly moving}}
+ (global) = mystd::move(global); // expected-warning{{explicitly moving}}
- using mystd::move;
- global = move(global); // expected-warning{{explicitly moving}} \
- expected-warning {{unqualified call to 'mystd::move}}
+ using mystd::move;
+ global = move(global); // expected-warning{{explicitly moving}} \
+ expected-warning {{unqualified call to 'mystd::move}}
}
class field_test {
@@ -75,16 +75,16 @@ class field_test {
};
class field_test_mystd {
- int x;
- field_test_mystd(field_test_mystd&& other) {
- x = mystd::move(x); // expected-warning{{explicitly moving}}
- x = mystd::move(other.x);
- other.x = mystd::move(x);
- other.x = mystd::move(other.x); // expected-warning{{explicitly moving}}
- }
- void withSuggest(int x) {
- x = mystd::move(x); // expected-warning{{explicitly moving variable of type 'int' to itself; did you mean to move to member 'x'?}}
- }
+ int x;
+ field_test_mystd(field_test_mystd&& other) {
+ x = mystd::move(x); // expected-warning{{explicitly moving}}
+ x = mystd::move(other.x);
+ other.x = mystd::move(x);
+ other.x = mystd::move(other.x); // expected-warning{{explicitly moving}}
+ }
+ void withSuggest(int x) {
+ x = mystd::move(x); // expected-warning{{explicitly moving variable of type 'int' to itself; did you mean to move to member 'x'?}}
+ }
};
struct A {};
@@ -103,13 +103,13 @@ void struct_test() {
}
void struct_test_mystd() {
- A a;
- a = mystd::move(a); // expected-warning{{explicitly moving}}
+ A a;
+ a = mystd::move(a); // expected-warning{{explicitly moving}}
- B b;
- b = mystd::move(b); // expected-warning{{explicitly moving}}
- b.a = mystd::move(b.a); // expected-warning{{explicitly moving}}
+ B b;
+ b = mystd::move(b); // expected-warning{{explicitly moving}}
+ b.a = mystd::move(b.a); // expected-warning{{explicitly moving}}
- C c;
- c = mystd::move(c); // expected-warning{{explicitly moving}}
+ C c;
+ c = mystd::move(c); // expected-warning{{explicitly moving}}
}
More information about the cfe-commits
mailing list