From 64c045e25b8471bbb572bd29159c294a82a86a25 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 31 Mar 2022 18:40:40 -0700 Subject: [PATCH] Treat `std::move`, `forward`, and `move_if_noexcept` as builtins. We still require these functions to be declared before they can be used, but don't instantiate their definitions unless their addresses are taken. Instead, code generation, constant evaluation, and static analysis are given direct knowledge of their effect. This change aims to reduce various costs associated with these functions -- per-instantiation memory costs, compile time and memory costs due to creating out-of-line copies and inlining them, code size at -O0, and so on -- so that they are not substantially more expensive than a cast. Most of these improvements are very small, but I measured a 3% decrease in -O0 object file size for a simple C++ source file using the standard library after this change. We now automatically infer the `const` and `nothrow` attributes on these now-builtin functions, in particular meaning that we get a warning for an unused call to one of these functions. In C++20 onwards, we disallow taking the addresses of these functions, per the C++20 "addressable function" rule. In earlier language modes, a compatibility warning is produced but the address can still be taken. The same infrastructure is extended to the existing MSVC builtin `__GetExceptionInfo`, which is now only recognized in namespace `std` like it always should have been. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D123345 --- clang/docs/CommandGuide/clang.rst | 20 ++++- clang/docs/ReleaseNotes.rst | 5 +- clang/include/clang/Basic/Builtins.def | 17 +++- clang/include/clang/Basic/Builtins.h | 25 +++++- clang/include/clang/Basic/DiagnosticSemaKinds.td | 9 +++ clang/lib/AST/ExprConstant.cpp | 14 ++++ clang/lib/Analysis/BodyFarm.cpp | 45 ++++++++++- clang/lib/Basic/Builtins.cpp | 36 ++++++--- clang/lib/CodeGen/CGBuiltin.cpp | 5 ++ clang/lib/CodeGen/CGCall.cpp | 2 + clang/lib/Sema/SemaChecking.cpp | 12 +++ clang/lib/Sema/SemaDecl.cpp | 61 ++++++++++----- clang/lib/Sema/SemaExpr.cpp | 35 ++++++++- clang/lib/Sema/SemaExprCXX.cpp | 8 ++ clang/lib/Sema/SemaInit.cpp | 4 + clang/lib/Sema/SemaOverload.cpp | 32 ++++---- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 6 ++ clang/test/Analysis/use-after-move.cpp | 4 +- clang/test/CodeGenCXX/builtin-std-move.cpp | 52 +++++++++++++ clang/test/CodeGenCXX/microsoft-abi-throw.cpp | 6 -- clang/test/SemaCXX/builtin-std-move-nobuiltin.cpp | 20 +++++ clang/test/SemaCXX/builtin-std-move.cpp | 90 ++++++++++++++++++++++ clang/test/SemaCXX/unqualified-std-call-fixits.cpp | 12 +-- clang/test/SemaCXX/unqualified-std-call.cpp | 6 +- clang/test/SemaCXX/warn-consumed-analysis.cpp | 6 +- 25 files changed, 456 insertions(+), 76 deletions(-) create mode 100644 clang/test/CodeGenCXX/builtin-std-move.cpp create mode 100644 clang/test/SemaCXX/builtin-std-move-nobuiltin.cpp create mode 100644 clang/test/SemaCXX/builtin-std-move.cpp diff --git a/clang/docs/CommandGuide/clang.rst b/clang/docs/CommandGuide/clang.rst index aec6278..658a304 100644 --- a/clang/docs/CommandGuide/clang.rst +++ b/clang/docs/CommandGuide/clang.rst @@ -252,8 +252,24 @@ Language Selection and Mode Options .. option:: -fno-builtin - Disable special handling and optimizations of builtin functions like - :c:func:`strlen` and :c:func:`malloc`. + Disable special handling and optimizations of well-known library functions, + like :c:func:`strlen` and :c:func:`malloc`. + +.. option:: -fno-builtin- + + Disable special handling and optimizations for the specific library function. + For example, ``-fno-builtin-strlen`` removes any special handling for the + :c:func:`strlen` library function. + +.. option:: -fno-builtin-std- + + Disable special handling and optimizations for the specific C++ standard + library function in namespace ``std``. For example, + ``-fno-builtin-std-move_if_noexcept`` removes any special handling for the + :cpp:func:`std::move_if_noexcept` library function. + + For C standard library functions that the C++ standard library also provides + in namespace ``std``, use :option:`-fno-builtin-\` instead. .. option:: -fmath-errno diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index c72028c..b2a5f6c3 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -230,7 +230,10 @@ C2x Feature Support C++ Language Changes in Clang ----------------------------- -- ... +- Improved ``-O0`` code generation for calls to ``std::move``, ``std::forward``, + and ``std::move_if_noexcept``. These are now treated as compiler builtins and + implemented directly, rather than instantiating the definition from the + standard library. C++20 Feature Support ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/Builtins.def b/clang/include/clang/Basic/Builtins.def index 62e82cd..c853229 100644 --- a/clang/include/clang/Basic/Builtins.def +++ b/clang/include/clang/Basic/Builtins.def @@ -81,7 +81,9 @@ // builtin even if type doesn't match signature, and don't warn if we // can't be sure the type is right // F -> this is a libc/libm function with a '__builtin_' prefix added. -// f -> this is a libc/libm function without the '__builtin_' prefix. +// f -> this is a libc/libm function without a '__builtin_' prefix, or with +// 'z', a C++ standard library function in namespace std::. This builtin +// is disableable by '-fno-builtin-foo' / '-fno-builtin-std-foo'. // h -> this function requires a specific header or an explicit declaration. // i -> this is a runtime library implemented function without the // '__builtin_' prefix. It will be implemented in compiler-rt or libgcc. @@ -101,6 +103,8 @@ // V:N: -> requires vectors of at least N bits to be legal // C -> callback behavior: argument N is called with argument // M_0, ..., M_k as payload +// z -> this is a C++ standard library function in (possibly-versioned) +// namespace std; implied by STDBUILTIN // FIXME: gcc has nonnull #if defined(BUILTIN) && !defined(LIBBUILTIN) @@ -111,6 +115,10 @@ # define LANGBUILTIN(ID, TYPE, ATTRS, BUILTIN_LANG) BUILTIN(ID, TYPE, ATTRS) #endif +#if defined(BUILTIN) && !defined(STDBUILTIN) +# define STDBUILTIN(ID, TYPE, ATTRS, HEADER) LIBBUILTIN(ID, TYPE, "zf" ATTRS, HEADER, CXX_LANG) +#endif + // Standard libc/libm functions: BUILTIN(__builtin_atan2 , "ddd" , "Fne") BUILTIN(__builtin_atan2f, "fff" , "Fne") @@ -919,7 +927,7 @@ LANGBUILTIN(__exception_info, "v*", "n", ALL_MS_LANGUAGES) LANGBUILTIN(_exception_info, "v*", "n", ALL_MS_LANGUAGES) LANGBUILTIN(__abnormal_termination, "i", "n", ALL_MS_LANGUAGES) LANGBUILTIN(_abnormal_termination, "i", "n", ALL_MS_LANGUAGES) -LANGBUILTIN(__GetExceptionInfo, "v*.", "ntu", ALL_MS_LANGUAGES) +LANGBUILTIN(__GetExceptionInfo, "v*.", "zntu", ALL_MS_LANGUAGES) LANGBUILTIN(_InterlockedAnd8, "ccD*c", "n", ALL_MS_LANGUAGES) LANGBUILTIN(_InterlockedAnd16, "ssD*s", "n", ALL_MS_LANGUAGES) LANGBUILTIN(_InterlockedAnd, "NiNiD*Ni", "n", ALL_MS_LANGUAGES) @@ -1543,6 +1551,11 @@ LIBBUILTIN(_Block_object_assign, "vv*vC*iC", "f", "Blocks.h", ALL_LANGUAGES) LIBBUILTIN(_Block_object_dispose, "vvC*iC", "f", "Blocks.h", ALL_LANGUAGES) // FIXME: Also declare NSConcreteGlobalBlock and NSConcreteStackBlock. +// C++11 +STDBUILTIN(move, "v&v&", "ncTh", "utility") +STDBUILTIN(move_if_noexcept, "v&v&", "ncTh", "utility") +STDBUILTIN(forward, "v&v&", "ncTh", "utility") + // Annotation function BUILTIN(__builtin_annotation, "v.", "tn") diff --git a/clang/include/clang/Basic/Builtins.h b/clang/include/clang/Basic/Builtins.h index 2926e0f..a82e157 100644 --- a/clang/include/clang/Basic/Builtins.h +++ b/clang/include/clang/Basic/Builtins.h @@ -138,6 +138,10 @@ public: /// Determines whether this builtin is a predefined libc/libm /// function, such as "malloc", where we know the signature a /// priori. + /// In C, such functions behave as if they are predeclared, + /// possibly with a warning on first use. In Objective-C and C++, + /// they do not, but they are recognized as builtins once we see + /// a declaration. bool isPredefinedLibFunction(unsigned ID) const { return strchr(getRecord(ID).Attributes, 'f') != nullptr; } @@ -156,6 +160,23 @@ public: return strchr(getRecord(ID).Attributes, 'i') != nullptr; } + /// Determines whether this builtin is a C++ standard library function + /// that lives in (possibly-versioned) namespace std, possibly a template + /// specialization, where the signature is determined by the standard library + /// declaration. + bool isInStdNamespace(unsigned ID) const { + return strchr(getRecord(ID).Attributes, 'z') != nullptr; + } + + /// Determines whether this builtin can have its address taken with no + /// special action required. + bool isDirectlyAddressable(unsigned ID) const { + // Most standard library functions can have their addresses taken. C++ + // standard library functions formally cannot in C++20 onwards, and when + // we allow it, we need to ensure we instantiate a definition. + return isPredefinedLibFunction(ID) && !isInStdNamespace(ID); + } + /// Determines whether this builtin has custom typechecking. bool hasCustomTypechecking(unsigned ID) const { return strchr(getRecord(ID).Attributes, 't') != nullptr; @@ -237,10 +258,6 @@ public: private: const Info &getRecord(unsigned ID) const; - /// Is this builtin supported according to the given language options? - bool builtinIsSupported(const Builtin::Info &BuiltinInfo, - const LangOptions &LangOpts); - /// Helper function for isPrintfLike and isScanfLike. bool isLike(unsigned ID, unsigned &FormatIdx, bool &HasVAListArg, const char *Fmt) const; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index e79a40d..cebd243 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6586,6 +6586,15 @@ def warn_self_move : Warning< "explicitly moving variable of type %0 to itself">, InGroup, DefaultIgnore; +def err_builtin_move_forward_unsupported : Error< + "unsupported signature for '%select{std::move|std::forward}0'">; +def err_use_of_unaddressable_function : Error< + "taking address of non-addressable standard library function">; +// FIXME: This should also be in -Wc++23-compat once we have it. +def warn_cxx20_compat_use_of_unaddressable_function : Warning< + "taking address of non-addressable standard library function " + "is incompatible with C++20">, InGroup; + def warn_redundant_move_on_return : Warning< "redundant move in return statement">, InGroup, DefaultIgnore; diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 6fe5d77..ce007ed 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -8127,6 +8127,7 @@ public: bool VisitVarDecl(const Expr *E, const VarDecl *VD); bool VisitUnaryPreIncDec(const UnaryOperator *UO); + bool VisitCallExpr(const CallExpr *E); bool VisitDeclRefExpr(const DeclRefExpr *E); bool VisitPredefinedExpr(const PredefinedExpr *E) { return Success(E); } bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E); @@ -8292,6 +8293,19 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) { return Success(*V, E); } +bool LValueExprEvaluator::VisitCallExpr(const CallExpr *E) { + switch (unsigned BuiltinOp = E->getBuiltinCallee()) { + case Builtin::BImove: + case Builtin::BImove_if_noexcept: + case Builtin::BIforward: + if (cast(E->getCalleeDecl())->isConstexpr()) + return Visit(E->getArg(0)); + break; + } + + return ExprEvaluatorBaseTy::VisitCallExpr(E); +} + bool LValueExprEvaluator::VisitMaterializeTemporaryExpr( const MaterializeTemporaryExpr *E) { // Walk through the expression to find the materialized temporary itself. diff --git a/clang/lib/Analysis/BodyFarm.cpp b/clang/lib/Analysis/BodyFarm.cpp index 92c236e..e8c2d44 100644 --- a/clang/lib/Analysis/BodyFarm.cpp +++ b/clang/lib/Analysis/BodyFarm.cpp @@ -20,6 +20,7 @@ #include "clang/AST/ExprObjC.h" #include "clang/AST/NestedNameSpecifier.h" #include "clang/Analysis/CodeInjector.h" +#include "clang/Basic/Builtins.h" #include "clang/Basic/OperatorKinds.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/Debug.h" @@ -86,6 +87,9 @@ public: ImplicitCastExpr *makeImplicitCast(const Expr *Arg, QualType Ty, CastKind CK = CK_LValueToRValue); + /// Create a cast to reference type. + CastExpr *makeReferenceCast(const Expr *Arg, QualType Ty); + /// Create an Objective-C bool literal. ObjCBoolLiteralExpr *makeObjCBool(bool Val); @@ -173,6 +177,16 @@ ImplicitCastExpr *ASTMaker::makeImplicitCast(const Expr *Arg, QualType Ty, /* FPFeatures */ FPOptionsOverride()); } +CastExpr *ASTMaker::makeReferenceCast(const Expr *Arg, QualType Ty) { + assert(Ty->isReferenceType()); + return CXXStaticCastExpr::Create( + C, Ty.getNonReferenceType(), + Ty->isLValueReferenceType() ? VK_LValue : VK_XValue, CK_NoOp, + const_cast(Arg), /*CXXCastPath=*/nullptr, + /*Written=*/C.getTrivialTypeSourceInfo(Ty), FPOptionsOverride(), + SourceLocation(), SourceLocation(), SourceRange()); +} + Expr *ASTMaker::makeIntegralCast(const Expr *Arg, QualType Ty) { if (Arg->getType() == Ty) return const_cast(Arg); @@ -296,6 +310,22 @@ static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M, /*FPFeatures=*/FPOptionsOverride()); } +/// Create a fake body for 'std::move' or 'std::forward'. This is just: +/// +/// \code +/// return static_cast(param); +/// \endcode +static Stmt *create_std_move_forward(ASTContext &C, const FunctionDecl *D) { + LLVM_DEBUG(llvm::dbgs() << "Generating body for std::move / std::forward\n"); + + ASTMaker M(C); + + QualType ReturnType = D->getType()->castAs()->getReturnType(); + Expr *Param = M.makeDeclRefExpr(D->getParamDecl(0)); + Expr *Cast = M.makeReferenceCast(Param, ReturnType); + return M.makeReturn(Cast); +} + /// Create a fake body for std::call_once. /// Emulates the following function body: /// @@ -681,8 +711,19 @@ Stmt *BodyFarm::getBody(const FunctionDecl *D) { FunctionFarmer FF; - if (Name.startswith("OSAtomicCompareAndSwap") || - Name.startswith("objc_atomicCompareAndSwap")) { + if (unsigned BuiltinID = D->getBuiltinID()) { + switch (BuiltinID) { + case Builtin::BImove: + case Builtin::BImove_if_noexcept: + case Builtin::BIforward: + FF = create_std_move_forward; + break; + default: + FF = nullptr; + break; + } + } else if (Name.startswith("OSAtomicCompareAndSwap") || + Name.startswith("objc_atomicCompareAndSwap")) { FF = create_OSAtomicCompareAndSwap; } else if (Name == "call_once" && D->getDeclContext()->isStdNamespace()) { FF = create_call_once; diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp index 6d278e9..ef8bb56 100644 --- a/clang/lib/Basic/Builtins.cpp +++ b/clang/lib/Basic/Builtins.cpp @@ -48,18 +48,22 @@ void Builtin::Context::InitializeTarget(const TargetInfo &Target, } bool Builtin::Context::isBuiltinFunc(llvm::StringRef FuncName) { - for (unsigned i = Builtin::NotBuiltin + 1; i != Builtin::FirstTSBuiltin; ++i) - if (FuncName.equals(BuiltinInfo[i].Name)) + bool InStdNamespace = FuncName.consume_front("std-"); + for (unsigned i = Builtin::NotBuiltin + 1; i != Builtin::FirstTSBuiltin; + ++i) { + if (FuncName.equals(BuiltinInfo[i].Name) && + (bool)strchr(BuiltinInfo[i].Attributes, 'z') == InStdNamespace) return strchr(BuiltinInfo[i].Attributes, 'f') != nullptr; + } return false; } -bool Builtin::Context::builtinIsSupported(const Builtin::Info &BuiltinInfo, - const LangOptions &LangOpts) { +/// Is this builtin supported according to the given language options? +static bool builtinIsSupported(const Builtin::Info &BuiltinInfo, + const LangOptions &LangOpts) { bool BuiltinsUnsupported = - (LangOpts.NoBuiltin || LangOpts.isNoBuiltinFunc(BuiltinInfo.Name)) && - strchr(BuiltinInfo.Attributes, 'f'); + LangOpts.NoBuiltin && strchr(BuiltinInfo.Attributes, 'f') != nullptr; bool CorBuiltinsUnsupported = !LangOpts.Coroutines && (BuiltinInfo.Langs & COR_LANG); bool MathBuiltinsUnsupported = @@ -111,6 +115,19 @@ void Builtin::Context::initializeBuiltins(IdentifierTable &Table, for (unsigned i = 0, e = AuxTSRecords.size(); i != e; ++i) Table.get(AuxTSRecords[i].Name) .setBuiltinID(i + Builtin::FirstTSBuiltin + TSRecords.size()); + + // Step #4: Unregister any builtins specified by -fno-builtin-foo. + for (llvm::StringRef Name : LangOpts.NoBuiltinFuncs) { + bool InStdNamespace = Name.consume_front("std-"); + auto NameIt = Table.find(Name); + if (NameIt != Table.end()) { + unsigned ID = NameIt->second->getBuiltinID(); + if (ID != Builtin::NotBuiltin && isPredefinedLibFunction(ID) && + isInStdNamespace(ID) == InStdNamespace) { + Table.get(Name).setBuiltinID(Builtin::NotBuiltin); + } + } + } } unsigned Builtin::Context::getRequiredVectorWidth(unsigned ID) const { @@ -190,8 +207,7 @@ bool Builtin::Context::performsCallback(unsigned ID, } bool Builtin::Context::canBeRedeclared(unsigned ID) const { - return ID == Builtin::NotBuiltin || - ID == Builtin::BI__va_start || - (!hasReferenceArgsOrResult(ID) && - !hasCustomTypechecking(ID)); + return ID == Builtin::NotBuiltin || ID == Builtin::BI__va_start || + (!hasReferenceArgsOrResult(ID) && !hasCustomTypechecking(ID)) || + isInStdNamespace(ID); } diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 3adabcb..dc4b104 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -4725,6 +4725,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, } break; + // C++ std:: builtins. + case Builtin::BImove: + case Builtin::BImove_if_noexcept: + case Builtin::BIforward: + return RValue::get(EmitLValue(E->getArg(0)).getPointer(*this)); case Builtin::BI__GetExceptionInfo: { if (llvm::GlobalVariable *GV = CGM.getCXXABI().getThrowInfo(FD->getParamDecl(0)->getType())) diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index e6a5fd1..d69455d 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -1805,6 +1805,8 @@ void CodeGenModule::getDefaultFunctionAttributes(StringRef Name, if (AttrOnCallSite) { // Attributes that should go on the call site only. + // FIXME: Look for 'BuiltinAttr' on the function rather than re-checking + // the -fno-builtin-foo list. if (!CodeGenOpts.SimplifyLibCalls || LangOpts.isNoBuiltinFunc(Name)) FuncAttrs.addAttribute(llvm::Attribute::NoBuiltin); if (!CodeGenOpts.TrapFuncName.empty()) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 03f9b69..870f133 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -2130,6 +2130,18 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, TheCall->setType(Context.VoidPtrTy); break; + case Builtin::BImove: + case Builtin::BImove_if_noexcept: + case Builtin::BIforward: + if (checkArgCount(*this, TheCall, 1)) + return ExprError(); + if (!Context.hasSameUnqualifiedType(TheCall->getType(), + TheCall->getArg(0)->getType())) { + Diag(TheCall->getBeginLoc(), diag::err_builtin_move_forward_unsupported) + << (BuiltinID == Builtin::BIforward); + return ExprError(); + } + break; // OpenCL v2.0, s6.13.16 - Pipe functions case Builtin::BIread_pipe: case Builtin::BIwrite_pipe: diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 499c70e..cacd08a 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -9269,6 +9269,29 @@ 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) { + switch (BuiltinID) { + case Builtin::BI__GetExceptionInfo: + // No type checking whatsoever. + return Ctx.getTargetInfo().getCXXABI().isMicrosoft(); + + case Builtin::BImove: + case Builtin::BImove_if_noexcept: + case Builtin::BIforward: { + // Ensure that we don't treat the algorithm + // OutputIt std::move(InputIt, InputIt, OutputIt) + // as the builtin std::move. + const auto *FPT = FD->getType()->castAs(); + return FPT->getNumParams() == 1 && !FPT->isVariadic(); + } + + default: + return false; + } +} + NamedDecl* Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, TypeSourceInfo *TInfo, LookupResult &Previous, @@ -10121,28 +10144,30 @@ 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() && - NewFD->getDeclContext()->getRedeclContext()->isFileContext()) { + if (!D.isRedeclaration()) { if (IdentifierInfo *II = Previous.getLookupName().getAsIdentifierInfo()) { if (unsigned BuiltinID = II->getBuiltinID()) { - if (NewFD->getLanguageLinkage() == CLanguageLinkage) { - // Validate the type matches unless this builtin is specified as - // matching regardless of its declared type. - if (Context.BuiltinInfo.allowTypeMismatch(BuiltinID)) { - NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID)); - } else { - ASTContext::GetBuiltinTypeError Error; - LookupNecessaryTypesForBuiltin(S, BuiltinID); - QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error); - - if (!Error && !BuiltinType.isNull() && - Context.hasSameFunctionTypeIgnoringExceptionSpec( - NewFD->getType(), BuiltinType)) + bool InStdNamespace = Context.BuiltinInfo.isInStdNamespace(BuiltinID); + 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. + if (Context.BuiltinInfo.allowTypeMismatch(BuiltinID)) { NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID)); + } else { + ASTContext::GetBuiltinTypeError Error; + LookupNecessaryTypesForBuiltin(S, BuiltinID); + QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error); + + if (!Error && !BuiltinType.isNull() && + Context.hasSameFunctionTypeIgnoringExceptionSpec( + NewFD->getType(), BuiltinType)) + NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID)); + } } - } else if (BuiltinID == Builtin::BI__GetExceptionInfo && - Context.getTargetInfo().getCXXABI().isMicrosoft()) { - // FIXME: We should consider this a builtin only in the std namespace. + } else if (InStdNamespace && NewFD->isInStdNamespace() && + isStdBuiltin(Context, NewFD, BuiltinID)) { NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID)); } } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 334ece6..cfebd3c 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -3395,7 +3395,7 @@ ExprResult Sema::BuildDeclarationNameExpr( case Decl::Function: { if (unsigned BID = cast(VD)->getBuiltinID()) { - if (!Context.BuiltinInfo.isPredefinedLibFunction(BID)) { + if (!Context.BuiltinInfo.isDirectlyAddressable(BID)) { type = Context.BuiltinFnTy; valueKind = VK_PRValue; break; @@ -20528,7 +20528,8 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) { auto *DRE = dyn_cast(E->IgnoreParenImpCasts()); if (DRE) { auto *FD = cast(DRE->getDecl()); - if (FD->getBuiltinID() == Builtin::BI__noop) { + unsigned BuiltinID = FD->getBuiltinID(); + if (BuiltinID == Builtin::BI__noop) { E = ImpCastExprToType(E, Context.getPointerType(FD->getType()), CK_BuiltinFnToFnPtr) .get(); @@ -20536,6 +20537,36 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) { VK_PRValue, SourceLocation(), FPOptionsOverride()); } + + if (Context.BuiltinInfo.isInStdNamespace(BuiltinID)) { + // Any use of these other than a direct call is ill-formed as of C++20, + // because they are not addressable functions. In earlier language + // modes, warn and force an instantiation of the real body. + Diag(E->getBeginLoc(), + getLangOpts().CPlusPlus20 + ? diag::err_use_of_unaddressable_function + : diag::warn_cxx20_compat_use_of_unaddressable_function); + if (FD->isImplicitlyInstantiable()) { + // Require a definition here because a normal attempt at + // instantiation for a builtin will be ignored, and we won't try + // again later. We assume that the definition of the template + // precedes this use. + InstantiateFunctionDefinition(E->getBeginLoc(), FD, + /*Recursive=*/false, + /*DefinitionRequired=*/true, + /*AtEndOfTU=*/false); + } + // Produce a properly-typed reference to the function. + CXXScopeSpec SS; + SS.Adopt(DRE->getQualifierLoc()); + TemplateArgumentListInfo TemplateArgs; + DRE->copyTemplateArgumentsInto(TemplateArgs); + return BuildDeclRefExpr( + FD, FD->getType(), VK_LValue, DRE->getNameInfo(), + DRE->hasQualifier() ? &SS : nullptr, DRE->getFoundDecl(), + DRE->getTemplateKeywordLoc(), + DRE->hasExplicitTemplateArgs() ? &TemplateArgs : nullptr); + } } Diag(E->getBeginLoc(), diag::err_builtin_fn_use); diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index aad9574..e765d1f 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -4236,6 +4236,14 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType, return ExprError(); From = FixOverloadedFunctionReference(From, Found, Fn); + + // We might get back another placeholder expression if we resolved to a + // builtin. + ExprResult Checked = CheckPlaceholderExpr(From); + if (Checked.isInvalid()) + return ExprError(); + + From = Checked.get(); FromType = From->getType(); } diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 233be54..f76fd92 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -8215,6 +8215,10 @@ ExprResult InitializationSequence::Perform(Sema &S, CurInit = S.FixOverloadedFunctionReference(CurInit, Step->Function.FoundDecl, Step->Function.Function); + // We might get back another placeholder expression if we resolved to a + // builtin. + if (!CurInit.isInvalid()) + CurInit = S.CheckPlaceholderExpr(CurInit.get()); break; case SK_CastDerivedToBasePRValue: diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 3327160..1add971 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -1747,13 +1747,6 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType, "Non-address-of operator for overloaded function expression"); FromType = S.Context.getPointerType(FromType); } - - // Check that we've computed the proper type after overload resolution. - // FIXME: FixOverloadedFunctionReference has side-effects; we shouldn't - // be calling it from within an NDEBUG block. - assert(S.Context.hasSameType( - FromType, - S.FixOverloadedFunctionReference(From, AccessPair, Fn)->getType())); } else { return false; } @@ -15188,10 +15181,9 @@ Expr *Sema::FixOverloadedFunctionReference(Expr *E, DeclAccessPair Found, if (SubExpr == UnOp->getSubExpr()) return UnOp; - return UnaryOperator::Create( - Context, SubExpr, UO_AddrOf, Context.getPointerType(SubExpr->getType()), - VK_PRValue, OK_Ordinary, UnOp->getOperatorLoc(), false, - CurFPFeatureOverrides()); + // FIXME: This can't currently fail, but in principle it could. + return CreateBuiltinUnaryOp(UnOp->getOperatorLoc(), UO_AddrOf, SubExpr) + .get(); } if (UnresolvedLookupExpr *ULE = dyn_cast(E)) { @@ -15202,10 +15194,20 @@ Expr *Sema::FixOverloadedFunctionReference(Expr *E, DeclAccessPair Found, TemplateArgs = &TemplateArgsBuffer; } - DeclRefExpr *DRE = - BuildDeclRefExpr(Fn, Fn->getType(), VK_LValue, ULE->getNameInfo(), - ULE->getQualifierLoc(), Found.getDecl(), - ULE->getTemplateKeywordLoc(), TemplateArgs); + QualType Type = Fn->getType(); + ExprValueKind ValueKind = getLangOpts().CPlusPlus ? VK_LValue : VK_PRValue; + + // FIXME: Duplicated from BuildDeclarationNameExpr. + if (unsigned BID = Fn->getBuiltinID()) { + if (!Context.BuiltinInfo.isDirectlyAddressable(BID)) { + Type = Context.BuiltinFnTy; + ValueKind = VK_PRValue; + } + } + + DeclRefExpr *DRE = BuildDeclRefExpr( + Fn, Type, ValueKind, ULE->getNameInfo(), ULE->getQualifierLoc(), + Found.getDecl(), ULE->getTemplateKeywordLoc(), TemplateArgs); DRE->setHadMultipleCandidates(ULE->getNumDecls() > 1); return DRE; } diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 9d0dc8c..8b0147f 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -4771,6 +4771,12 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, if (TSK == TSK_ExplicitSpecialization) return; + // Never implicitly instantiate a builtin; we don't actually need a function + // body. + if (Function->getBuiltinID() && TSK == TSK_ImplicitInstantiation && + !DefinitionRequired) + return; + // Don't instantiate a definition if we already have one. const FunctionDecl *ExistingDefn = nullptr; if (Function->isDefined(ExistingDefn, diff --git a/clang/test/Analysis/use-after-move.cpp b/clang/test/Analysis/use-after-move.cpp index d1278ca..ce9b26f 100644 --- a/clang/test/Analysis/use-after-move.cpp +++ b/clang/test/Analysis/use-after-move.cpp @@ -244,7 +244,7 @@ void reinitializationTest(int i) { A a; if (i == 1) { // peaceful-note 2 {{'i' is not equal to 1}} // peaceful-note@-1 2 {{Taking false branch}} - std::move(a); + (void)std::move(a); } if (i == 2) { // peaceful-note 2 {{'i' is not equal to 2}} // peaceful-note@-1 2 {{Taking false branch}} @@ -494,7 +494,7 @@ void templateArgIsNotUseTest() { // Moves of global variables are not reported. A global_a; void globalVariablesTest() { - std::move(global_a); + (void)std::move(global_a); global_a.foo(); // no-warning } diff --git a/clang/test/CodeGenCXX/builtin-std-move.cpp b/clang/test/CodeGenCXX/builtin-std-move.cpp new file mode 100644 index 0000000..29107c4 --- /dev/null +++ b/clang/test/CodeGenCXX/builtin-std-move.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -triple=x86_64-linux-gnu -emit-llvm -o - -std=c++17 %s | FileCheck %s --implicit-check-not=@_ZSt4move + +namespace std { + template constexpr T &&move(T &val) { return static_cast(val); } + template constexpr T &&move_if_noexcept(T &val); + template constexpr T &&forward(T &val); + + // Not the builtin. + template T move(U source, U source_end, T dest); +} + +class T {}; +extern "C" void take(T &&); + +T a; + +// Check emission of a constant-evaluated call. +// CHECK-DAG: @move_a = constant %[[T:.*]]* @a +T &&move_a = std::move(a); +// CHECK-DAG: @move_if_noexcept_a = constant %[[T]]* @a +T &&move_if_noexcept_a = std::move_if_noexcept(a); +// CHECK-DAG: @forward_a = constant %[[T]]* @a +T &forward_a = std::forward(a); + +// Check emission of a non-constant call. +// CHECK-LABEL: define {{.*}} void @test +extern "C" void test(T &t) { + // CHECK: store %[[T]]* %{{.*}}, %[[T]]** %[[T_REF:[^,]*]] + // CHECK: %0 = load %[[T]]*, %[[T]]** %[[T_REF]] + // CHECK: call void @take(%[[T]]* {{.*}} %0) + take(std::move(t)); + // CHECK: %1 = load %[[T]]*, %[[T]]** %[[T_REF]] + // CHECK: call void @take(%[[T]]* {{.*}} %1) + take(std::move_if_noexcept(t)); + // CHECK: %2 = load %[[T]]*, %[[T]]** %[[T_REF]] + // CHECK: call void @take(%[[T]]* {{.*}} %2) + take(std::forward(t)); + + // CHECK: call {{.*}} @_ZSt4moveI1TS0_ET_T0_S2_S1_ + std::move(t, t, t); +} + +// CHECK: declare {{.*}} @_ZSt4moveI1TS0_ET_T0_S2_S1_ + +// Check that we instantiate and emit if the address is taken. +// CHECK-LABEL: define {{.*}} @use_address +extern "C" void *use_address() { + // CHECK: ret {{.*}} @_ZSt4moveIiEOT_RS0_ + return (void*)&std::move; +} + +// CHECK: define {{.*}} i32* @_ZSt4moveIiEOT_RS0_(i32* diff --git a/clang/test/CodeGenCXX/microsoft-abi-throw.cpp b/clang/test/CodeGenCXX/microsoft-abi-throw.cpp index 9284959..aa79935 100644 --- a/clang/test/CodeGenCXX/microsoft-abi-throw.cpp +++ b/clang/test/CodeGenCXX/microsoft-abi-throw.cpp @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -o - -triple=i386-pc-win32 -std=c++11 %s -fcxx-exceptions -fms-extensions | FileCheck %s -// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -o - -triple=i386-pc-win32 -std=c++11 %s -fcxx-exceptions -fms-extensions -DSTD | FileCheck %s // CHECK-DAG: @"??_R0?AUY@@@8" = linkonce_odr global %rtti.TypeDescriptor7 { i8** @"??_7type_info@@6B@", i8* null, [8 x i8] c".?AUY@@\00" }, comdat // CHECK-DAG: @"_CT??_R0?AUY@@@8??0Y@@QAE@ABU0@@Z8" = linkonce_odr unnamed_addr constant %eh.CatchableType { i32 4, i8* bitcast (%rtti.TypeDescriptor7* @"??_R0?AUY@@@8" to i8*), i32 0, i32 -1, i32 0, i32 8, i8* bitcast (%struct.Y* (%struct.Y*, %struct.Y*, i32)* @"??0Y@@QAE@ABU0@@Z" to i8*) }, section ".xdata", comdat @@ -134,15 +133,10 @@ void h() { throw nullptr; } -#ifdef STD namespace std { template void *__GetExceptionInfo(T); } -#else -template -void *__GetExceptionInfo(T); -#endif using namespace std; void *GetExceptionInfo_test0() { diff --git a/clang/test/SemaCXX/builtin-std-move-nobuiltin.cpp b/clang/test/SemaCXX/builtin-std-move-nobuiltin.cpp new file mode 100644 index 0000000..9185bd8 --- /dev/null +++ b/clang/test/SemaCXX/builtin-std-move-nobuiltin.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s -DBUILTIN=builtin +// RUN: %clang_cc1 -std=c++20 -verify %s -DBUILTIN=nobuiltin -fno-builtin +// RUN: %clang_cc1 -std=c++20 -verify %s -DBUILTIN=nobuiltin -fno-builtin-std-move -fno-builtin-std-move_if_noexcept -fno-builtin-std-forward +// RUN: %clang_cc1 -std=c++20 -verify %s -DBUILTIN=nobuiltin -ffreestanding +// expected-no-diagnostics + +int nobuiltin; + +namespace std { + template constexpr T &&move(T &x) { return (T&&)nobuiltin; } + template constexpr T &&move_if_noexcept(T &x) { return (T&&)nobuiltin; } + template constexpr T &&forward(T &x) { return (T&&)nobuiltin; } +} + +template constexpr T *addr(T &&r) { return &r; } + +int builtin; +static_assert(addr(std::move(builtin)) == &BUILTIN); +static_assert(addr(std::move_if_noexcept(builtin)) == &BUILTIN); +static_assert(addr(std::forward(builtin)) == &BUILTIN); diff --git a/clang/test/SemaCXX/builtin-std-move.cpp b/clang/test/SemaCXX/builtin-std-move.cpp new file mode 100644 index 0000000..6a26323 --- /dev/null +++ b/clang/test/SemaCXX/builtin-std-move.cpp @@ -0,0 +1,90 @@ +// RUN: %clang_cc1 -std=c++17 -verify %s +// RUN: %clang_cc1 -std=c++17 -verify %s -DNO_CONSTEXPR +// RUN: %clang_cc1 -std=c++20 -verify %s + +namespace std { +#ifndef NO_CONSTEXPR +#define CONSTEXPR constexpr +#else +#define CONSTEXPR +#endif + + template CONSTEXPR T &&move(T &x) { + static_assert(T::moveable, "instantiated move"); // expected-error {{no member named 'moveable' in 'B'}} + // expected-error@-1 {{no member named 'moveable' in 'C'}} + return static_cast(x); + } + + // Unrelated move functions are not the builtin. + template CONSTEXPR int move(T, T) { return 5; } + + template struct ref { using type = T&; }; + template struct ref { using type = T&&; }; + + template CONSTEXPR auto move_if_noexcept(T &x) -> typename ref(x)))>::type { + static_assert(T::moveable, "instantiated move_if_noexcept"); // expected-error {{no member named 'moveable' in 'B'}} + return static_cast(x)))>::type>(x); + } + + template struct remove_reference { using type = T; }; + template struct remove_reference { using type = T; }; + template struct remove_reference { using type = T; }; + + template CONSTEXPR T &&forward(typename remove_reference::type &x) { + static_assert(T::moveable, "instantiated forward"); // expected-error {{no member named 'moveable' in 'B'}} + // expected-error@-1 {{no member named 'moveable' in 'C'}} + return static_cast(x); + } +} + +// Note: this doesn't have a 'moveable' member. Instantiation of the above +// functions will fail if it's attempted. +struct A {}; +constexpr bool f(A a) { // #f + A &&move = std::move(a); // #call + A &&move_if_noexcept = std::move_if_noexcept(a); + A &&forward1 = std::forward(a); + A &forward2 = std::forward(a); + return &move == &a && &move_if_noexcept == &a && + &forward1 == &a && &forward2 == &a && + std::move(a, a) == 5; +} + +#ifndef NO_CONSTEXPR +static_assert(f({}), "should be constexpr"); +#else +// expected-error@#f {{never produces a constant expression}} +// expected-note@#call {{}} +#endif + +struct B {}; +B &&(*pMove)(B&) = std::move; // #1 expected-note {{instantiation of}} +B &&(*pMoveIfNoexcept)(B&) = &std::move_if_noexcept; // #2 expected-note {{instantiation of}} +B &&(*pForward)(B&) = &std::forward; // #3 expected-note {{instantiation of}} +int (*pUnrelatedMove)(B, B) = std::move; + +struct C {}; +C &&(&rMove)(C&) = std::move; // #4 expected-note {{instantiation of}} +C &&(&rForward)(C&) = std::forward; // #5 expected-note {{instantiation of}} +int (&rUnrelatedMove)(B, B) = std::move; + +#if __cplusplus <= 201703L +// expected-warning@#1 {{non-addressable}} +// expected-warning@#2 {{non-addressable}} +// expected-warning@#3 {{non-addressable}} +// expected-warning@#4 {{non-addressable}} +// expected-warning@#5 {{non-addressable}} +#else +// expected-error@#1 {{non-addressable}} +// expected-error@#2 {{non-addressable}} +// expected-error@#3 {{non-addressable}} +// expected-error@#4 {{non-addressable}} +// expected-error@#5 {{non-addressable}} +#endif + +void attribute_const() { + int n; + std::move(n); // expected-warning {{ignoring return value}} + std::move_if_noexcept(n); // expected-warning {{ignoring return value}} + std::forward(n); // expected-warning {{ignoring return value}} +} diff --git a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp b/clang/test/SemaCXX/unqualified-std-call-fixits.cpp index 0b2d70c..d6f8e5e 100644 --- a/clang/test/SemaCXX/unqualified-std-call-fixits.cpp +++ b/clang/test/SemaCXX/unqualified-std-call-fixits.cpp @@ -6,9 +6,9 @@ namespace std { -void move(auto &&a) {} +int &&move(auto &&a) { return a; } -void forward(auto &a) {} +int &&forward(auto &a) { return a; } } // namespace std @@ -16,8 +16,8 @@ using namespace std; void f() { int i = 0; - move(i); // expected-warning {{unqualified call to std::move}} - // CHECK: {{^}} std:: - forward(i); // expected-warning {{unqualified call to std::forward}} - // CHECK: {{^}} std:: + (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 } diff --git a/clang/test/SemaCXX/unqualified-std-call.cpp b/clang/test/SemaCXX/unqualified-std-call.cpp index fa66ae9..0c78c26 100644 --- a/clang/test/SemaCXX/unqualified-std-call.cpp +++ b/clang/test/SemaCXX/unqualified-std-call.cpp @@ -1,17 +1,17 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wall -std=c++11 %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wall -std=c++11 %s -Wno-unused-value namespace std { template void dummy(T &&) {} template -void move(T &&) {} +T &&move(T &&x) { return x; } template void move(T &&, U &&) {} inline namespace __1 { template -void forward(T &) {} +T &forward(T &x) { return x; } } // namespace __1 struct foo {}; diff --git a/clang/test/SemaCXX/warn-consumed-analysis.cpp b/clang/test/SemaCXX/warn-consumed-analysis.cpp index b4dddb6..8e09779 100644 --- a/clang/test/SemaCXX/warn-consumed-analysis.cpp +++ b/clang/test/SemaCXX/warn-consumed-analysis.cpp @@ -953,12 +953,12 @@ void test6() { namespace std { void move(); template - void move(T&&); + T &&move(T&); namespace __1 { void move(); template - void move(T&&); + T &&move(T&); } } @@ -971,7 +971,7 @@ namespace PR18260 { void test() { x.move(); std::move(); - std::move(x); + std::move(x); // expected-warning {{ignoring return value}} std::__1::move(); std::__1::move(x); } -- 2.7.4