llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Hans Wennborg (zmodem) <details> <summary>Changes</summary> Let's revert this until there's a plan in place for how code is supposed to transition to the new spelling. Reverts llvm/llvm-project#<!-- -->196635 --- Patch is 27.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/208691.diff 10 Files Affected: - (modified) clang/docs/ReleaseNotes.md (-9) - (modified) clang/include/clang/Basic/Attr.td (+1-11) - (modified) clang/include/clang/Basic/AttrDocs.td (+11-23) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3-8) - (modified) clang/lib/Sema/CheckExprLifetime.cpp (+6-8) - (modified) clang/lib/Sema/SemaChecking.cpp (+2-3) - (modified) clang/lib/Sema/SemaDeclAttr.cpp (+16-85) - (modified) clang/test/Sema/LifetimeSafety/capture-by.cpp (+11-23) - (modified) clang/test/Sema/LifetimeSafety/safety.cpp (+2-2) - (modified) clang/test/SemaCXX/attr-lifetime-capture-by.cpp (+10-42) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 449a3792eb4ba..7676699a9b54d 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -554,15 +554,6 @@ latest release, please see the [Clang Web Site](https://clang.llvm.org) or the - The `modular_format` attribute now supports the `fixed` aspect for C ISO 18037 fixed-point `printf` specifiers. -- The `lifetime_capture_by` attribute now accepts three new spelling forms: - `lifetime_capture_by_this`, `lifetime_capture_by_global`, and - `lifetime_capture_by_unknown`. These replace passing `this`, `global`, and - `unknown` as arguments to `lifetime_capture_by`; that argument form is now - deprecated because those names can conflict with user-defined parameters. - They will be removed in the next release. Distinct `lifetime_capture_by` - spellings may also be combined on the same declaration, but each spelling may - appear at most once. - - The `const` and `pure` attributes only apply to functions; they are now diagnosed and ignored when applied to anything else. diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 1a5bd2301dfc8..de3e0f7b563a7 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2137,19 +2137,9 @@ def LifetimeBound : DeclOrTypeAttr { } def LifetimeCaptureBy : DeclOrTypeAttr { - let Spellings = [Clang<"lifetime_capture_by">, - Clang<"lifetime_capture_by_this">, - Clang<"lifetime_capture_by_global">, - Clang<"lifetime_capture_by_unknown">]; + let Spellings = [Clang<"lifetime_capture_by", 1>]; let Subjects = SubjectList<[ParmVar, ImplicitObjectParameter], ErrorDiag>; let Args = [VariadicParamOrParamIdxArgument<"Params">]; - let Accessors = [Accessor<"isThis", [Clang<"lifetime_capture_by_this">]>, - Accessor<"isGlobal", [Clang<"lifetime_capture_by_global">]>, - Accessor<"isUnknown", [Clang<"lifetime_capture_by_unknown">]>, - Accessor<"isStandaloneSpecial", - [Clang<"lifetime_capture_by_this">, - Clang<"lifetime_capture_by_global">, - Clang<"lifetime_capture_by_unknown">]>]; let Documentation = [LifetimeCaptureByDocs]; let AdditionalMembers = [{ private: diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 05e4cb0870652..ba19e61168575 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -4701,13 +4701,10 @@ have their lifetimes extended. def LifetimeCaptureByDocs : Documentation { let Category = DocCatFunction; - let Heading = "lifetime_capture_by, lifetime_capture_by_this, lifetime_capture_by_global, lifetime_capture_by_unknown"; let Content = [{ -Similar to `lifetimebound`_, the ``lifetime_capture_by`` attribute family on a -function parameter or implicit object parameter indicates that a capturing -entity may refer to the object referred to by that parameter. The capturing -entity can be named in ``lifetime_capture_by(X)`` or selected by one of the -standalone special forms listed below. +Similar to `lifetimebound`_, the ``lifetime_capture_by(X)`` attribute on a +function parameter or implicit object parameter indicates that the capturing +entity ``X`` may refer to the object referred by that parameter. Below is a list of types of the parameters and what they're considered to refer to: @@ -4739,7 +4736,7 @@ temporary object. addToSet(local, s); // Ok. } -The capturing entity can be one of the following: +The capturing entity ``X`` can be one of the following: - Another (named) function parameter. @@ -4749,30 +4746,28 @@ The capturing entity can be one of the following: s.insert(a); } -- ``this`` (in case of member functions), written as - ``lifetime_capture_by_this``. +- ``this`` (in case of member functions). .. code-block:: c++ class S { - void addToSet(std::string_view a [[clang::lifetime_capture_by_this]]) { + void addToSet(std::string_view a [[clang::lifetime_capture_by(this)]]) { s.insert(a); } std::set<std::string_view> s; }; - Note: When applied to a constructor parameter, `[[clang::lifetime_capture_by_this]]` is just an alias of `[[clang::lifetimebound]]`. + Note: When applied to a constructor parameter, `[[clang::lifetime_capture_by(this)]]` is just an alias of `[[clang::lifetimebound]]`. -- ``global`` and ``unknown``, written as ``lifetime_capture_by_global`` and - ``lifetime_capture_by_unknown`` respectively. +- `global`, `unknown`. .. code-block:: c++ std::set<std::string_view> s; - void addToSet(std::string_view a [[clang::lifetime_capture_by_global]]) { + void addToSet(std::string_view a [[clang::lifetime_capture_by(global)]]) { s.insert(a); } - void addSomewhere(std::string_view a [[clang::lifetime_capture_by_unknown]]); + void addSomewhere(std::string_view a [[clang::lifetime_capture_by(unknown)]]); The attribute can be applied to the implicit ``this`` parameter of a member function by writing the attribute after the function type: @@ -4785,7 +4780,7 @@ function by writing the attribute after the function type: } }; -The parameter-list form supports specifying more than one capturing entity: +The attribute supports specifying more than one capturing entities: .. code-block:: c++ @@ -4796,13 +4791,6 @@ The parameter-list form supports specifying more than one capturing entity: s2.insert(a); } -Distinct ``lifetime_capture_by`` forms can also be combined on the same -declaration, but each form can appear at most once. For example, -``[[clang::lifetime_capture_by(s), clang::lifetime_capture_by_this]]`` is -allowed, but two ``[[clang::lifetime_capture_by(...)]]`` attributes or two -``[[clang::lifetime_capture_by_this]]`` attributes on the same declaration are -rejected. - Limitation: The capturing entity ``X`` is not used by the analysis and is used for documentation purposes only. This is because the analysis is statement-local and only detects use of a temporary as an argument to the diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 09951730e1570..e57f3e0e14168 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3566,23 +3566,18 @@ def err_callback_callee_is_variadic : Error< def err_callback_implicit_this_not_available : Error< "'callback' argument at position %0 references unavailable implicit 'this'">; +def err_capture_by_attribute_multiple : Error< + "multiple 'lifetime_capture' attributes specified">; def err_capture_by_attribute_no_entity : Error< "'lifetime_capture_by' attribute specifies no capturing entity">; def err_capture_by_implicit_this_not_available : Error< "'lifetime_capture_by' argument references unavailable implicit 'this'">; -def err_capture_by_this_attr_without_implicit_this : Error< - "'lifetime_capture_by_this' attribute requires an implicit object parameter">; def err_capture_by_attribute_argument_unknown : Error< "'lifetime_capture_by' attribute argument %0 is not a known function parameter" "; must be a function parameter, 'this', 'global' or 'unknown'">; def err_capture_by_references_itself : Error<"'lifetime_capture_by' argument references itself">; def err_capture_by_param_uses_reserved_name : Error< - "parameter cannot be named '%0' while using 'lifetime_capture_by(%0)'">; -def err_capture_by_attribute_multiple : Error< - "multiple '%0' attributes specified">; -def warn_deprecated_capture_by_special_entity : Warning< - "'lifetime_capture_by(%0)' is deprecated; use '%1' instead">, - InGroup<DeprecatedAttributes>; + "parameter cannot be named '%select{global|unknown}0' while using 'lifetime_capture_by(%select{global|unknown}0)'">; def err_init_method_bad_return_type : Error< "init methods must return an object pointer type, not %0">; diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index fafee76ec18c3..d15b2c6518cec 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -502,14 +502,12 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, if (CheckCoroCall || CanonCallee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>()) VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg); - else if (isa<CXXConstructorDecl>(CanonCallee) && - llvm::any_of(CanonCallee->getParamDecl(I) - ->specific_attrs<LifetimeCaptureByAttr>(), - [](const LifetimeCaptureByAttr *CaptureAttr) { - return llvm::is_contained( - CaptureAttr->params(), - LifetimeCaptureByAttr::This); - })) + else if (const auto *CaptureAttr = + CanonCallee->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>(); + CaptureAttr && isa<CXXConstructorDecl>(CanonCallee) && + llvm::any_of(CaptureAttr->params(), [](int ArgIdx) { + return ArgIdx == LifetimeCaptureByAttr::This; + })) // `lifetime_capture_by(this)` in a class constructor has the same // semantics as `lifetimebound`: // diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 2a8395e26d0bd..79fec78f88c69 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -4405,9 +4405,8 @@ void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction, } }; for (unsigned I = 0; I < FD->getNumParams(); ++I) - for (const auto *A : - FD->getParamDecl(I)->specific_attrs<LifetimeCaptureByAttr>()) - HandleCaptureByAttr(A, I + IsMemberFunction); + HandleCaptureByAttr(FD->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>(), + I + IsMemberFunction); // Check when the implicit object param is captured. if (IsMemberFunction) { TypeSourceInfo *TSI = FD->getTypeSourceInfo(); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 1b272b5416860..1300635921d5e 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -4411,41 +4411,17 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) { LifetimeCaptureByAttr *Sema::ParseLifetimeCaptureByAttr(const ParsedAttr &AL, StringRef ParamName) { - StringRef AttrName = AL.getAttrName()->getName(); - StringRef SpecialEntity; - if (AttrName == "lifetime_capture_by_this") - SpecialEntity = "this"; - else if (AttrName == "lifetime_capture_by_global") - SpecialEntity = "global"; - else if (AttrName == "lifetime_capture_by_unknown") - SpecialEntity = "unknown"; - - if (!SpecialEntity.empty() && AL.getNumArgs() != 0) { - Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 0; - return nullptr; - } - // Atleast one capture by is required. - if (SpecialEntity.empty() && AL.getNumArgs() == 0) { + if (AL.getNumArgs() == 0) { Diag(AL.getLoc(), diag::err_capture_by_attribute_no_entity) << AL.getRange(); return nullptr; } - unsigned N = SpecialEntity.empty() ? AL.getNumArgs() : 1; + unsigned N = AL.getNumArgs(); auto ParamIdents = MutableArrayRef<IdentifierInfo *>(new (Context) IdentifierInfo *[N], N); auto ParamLocs = MutableArrayRef<SourceLocation>(new (Context) SourceLocation[N], N); - if (!SpecialEntity.empty()) { - ParamIdents[0] = &Context.Idents.get(SpecialEntity); - ParamLocs[0] = AL.getRange().getEnd(); - int FakeParamIndices[] = {LifetimeCaptureByAttr::Invalid}; - auto *CapturedBy = - LifetimeCaptureByAttr::Create(Context, FakeParamIndices, 1, AL); - CapturedBy->setArgs(ParamIdents, ParamLocs); - return CapturedBy; - } - bool IsValid = true; for (unsigned I = 0; I < N; ++I) { if (AL.isArgExpr(I)) { @@ -4457,17 +4433,6 @@ LifetimeCaptureByAttr *Sema::ParseLifetimeCaptureByAttr(const ParsedAttr &AL, } assert(AL.isArgIdent(I)); IdentifierLoc *IdLoc = AL.getArgAsIdent(I); - StringRef Name = IdLoc->getIdentifierInfo()->getName(); - StringRef Replacement; - if (Name == "this") - Replacement = "lifetime_capture_by_this"; - else if (Name == "global") - Replacement = "lifetime_capture_by_global"; - else if (Name == "unknown") - Replacement = "lifetime_capture_by_unknown"; - if (!Replacement.empty()) - Diag(IdLoc->getLoc(), diag::warn_deprecated_capture_by_special_entity) - << Name << Replacement << IdLoc->getLoc(); if (IdLoc->getIdentifierInfo()->getName() == ParamName) { Diag(IdLoc->getLoc(), diag::err_capture_by_references_itself) << IdLoc->getLoc(); @@ -4488,53 +4453,24 @@ LifetimeCaptureByAttr *Sema::ParseLifetimeCaptureByAttr(const ParsedAttr &AL, static void handleLifetimeCaptureByAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + // Do not allow multiple attributes. + if (D->hasAttr<LifetimeCaptureByAttr>()) { + S.Diag(AL.getLoc(), diag::err_capture_by_attribute_multiple) + << AL.getRange(); + return; + } auto *PVD = dyn_cast<ParmVarDecl>(D); assert(PVD); auto *CaptureByAttr = S.ParseLifetimeCaptureByAttr(AL, PVD->getName()); - if (!CaptureByAttr) - return; - - enum class SpellingKind { ParameterList, This, Global, Unknown }; - auto GetSpellingKind = [](const LifetimeCaptureByAttr *A) { - if (A->isThis()) - return SpellingKind::This; - if (A->isGlobal()) - return SpellingKind::Global; - if (A->isUnknown()) - return SpellingKind::Unknown; - return SpellingKind::ParameterList; - }; - auto GetSpellingName = [](SpellingKind Kind) -> StringRef { - switch (Kind) { - case SpellingKind::ParameterList: - return "lifetime_capture_by"; - case SpellingKind::This: - return "lifetime_capture_by_this"; - case SpellingKind::Global: - return "lifetime_capture_by_global"; - case SpellingKind::Unknown: - return "lifetime_capture_by_unknown"; - } - llvm_unreachable("unknown lifetime_capture_by spelling kind"); - }; - - SpellingKind NewKind = GetSpellingKind(CaptureByAttr); - for (const auto *Existing : D->specific_attrs<LifetimeCaptureByAttr>()) { - if (GetSpellingKind(Existing) == NewKind) { - S.Diag(AL.getLoc(), diag::err_capture_by_attribute_multiple) - << GetSpellingName(NewKind) << AL.getRange(); - return; - } - } - - D->addAttr(CaptureByAttr); + if (CaptureByAttr) + D->addAttr(CaptureByAttr); } void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) { bool HasImplicitThisParam = hasImplicitObjectParameter(FD); SmallVector<LifetimeCaptureByAttr *, 1> Attrs; for (ParmVarDecl *PVD : FD->parameters()) - for (auto *A : PVD->specific_attrs<LifetimeCaptureByAttr>()) + if (auto *A = PVD->getAttr<LifetimeCaptureByAttr>()) Attrs.push_back(A); if (HasImplicitThisParam) { TypeSourceInfo *TSI = FD->getTypeSourceInfo(); @@ -4564,7 +4500,7 @@ void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) { for (const ParmVarDecl *PVD : FD->parameters()) if (PVD->getName() == Reserved) Diag(PVD->getLocation(), diag::err_capture_by_param_uses_reserved_name) - << PVD->getName(); + << (PVD->getName() == "unknown"); }; for (auto *CapturedBy : Attrs) { const auto &Entities = CapturedBy->getArgIdents(); @@ -4573,19 +4509,14 @@ void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) { auto It = NameIdxMapping.find(Name); if (It == NameIdxMapping.end()) { auto Loc = CapturedBy->getArgLocs()[I]; - if (!HasImplicitThisParam && Name == "this") { - unsigned DiagID = - CapturedBy->isStandaloneSpecial() - ? diag::err_capture_by_this_attr_without_implicit_this - : diag::err_capture_by_implicit_this_not_available; - Diag(Loc, DiagID) << Loc; - } else + if (!HasImplicitThisParam && Name == "this") + Diag(Loc, diag::err_capture_by_implicit_this_not_available) << Loc; + else Diag(Loc, diag::err_capture_by_attribute_argument_unknown) << Entities[I] << Loc; continue; } - if ((Name == "unknown" || Name == "global") && - !CapturedBy->isStandaloneSpecial()) + if (Name == "unknown" || Name == "global") DisallowReservedParams(Name); CapturedBy->setParamIdx(I, It->second); } diff --git a/clang/test/Sema/LifetimeSafety/capture-by.cpp b/clang/test/Sema/LifetimeSafety/capture-by.cpp index e81669fe30782..2877d8d6cd5f9 100644 --- a/clang/test/Sema/LifetimeSafety/capture-by.cpp +++ b/clang/test/Sema/LifetimeSafety/capture-by.cpp @@ -85,18 +85,6 @@ void use() { } } // namespace capture_string_view -namespace multiple_capture_by_attrs { -struct X {} x1; -void capture(std::string_view s [[clang::lifetime_capture_by(x1), - clang::lifetime_capture_by_global]], - X &x1); - -void use() { - capture(std::string(), // expected-warning {{object whose reference is captured by 'x1' will be destroyed at the end of the full-expression}} expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} - x1); -} -} // namespace multiple_capture_by_attrs - // **************************************************************************** // Capture pointer (eg: std::string*) // **************************************************************************** @@ -157,7 +145,7 @@ void use() { namespace temporary_capturing_object { struct S { - void add(const int& x [[clang::lifetime_capture_by_this]]); + void add(const int& x [[clang::lifetime_capture_by(this)]]); }; void test() { @@ -173,8 +161,8 @@ void test() { // Capture by Global and Unknown. // **************************************************************************** namespace capture_by_global_unknown { -void captureByGlobal(std::string_view s [[clang::lifetime_capture_by_global]]); -void captureByUnknown(std::string_view s [[clang::lifetime_capture_by_unknown]]); +void captureByGlobal(std::string_view s [[clang::lifetime_capture_by(global)]]); +void captureByUnknown(std::string_view s [[clang::lifetime_capture_by(unknown)]]); std::string_view getLifetimeBoundView(const std::string& s [[clang::lifetimebound]]); @@ -200,8 +188,8 @@ void use() { // **************************************************************************** namespace capture_by_this { struct S { - void captureInt(const int& x [[clang::lifetime_capture_by_this]]); - void captureView(std::string_view sv [[clang::lifetime_capture_by_this]]); + void captureInt(const int& x [[clang::lifetime_capture_by(this)]]); + void captureView(std::string_view sv [[clang::lifetime_capture_by(this)]]); }; std::string_view getLifetimeBoundView(const std::string& s [[clang::lifetimebound]]); std::string_view getNotLifetimeBoundView(const std::string& s); @@ -260,8 +248,8 @@ void useCaptureDefaultArg() { namespace containers_no_distinction { template<class T> struct MySet { - void insert(T&& t [[clang::lifetime_capture_by_this]]); - void insert(const T& t [[clang::lifetime_capture_by_this]]); + void insert(T&& t [[clang::lifetime_capture_by(this)]]); + void insert(const T& t [[clang::lifetime_capture_by(this)]]); }; void user_defined_containers() { MySet<int> set_of_int; @@ -281,8 +269,8 @@ template<> struct IsPointerLikeTypeImpl<std::string_view> : std::true_type {}; template<typename T> concept IsPointerLikeType = std::is_pointer<T>::value || IsPointerLikeTypeImpl<T>::value; template<class T> struct MyVector { - void push_back(T&& t [[clang::lifetime_capture_by_this]]) requires IsPointerLikeType<T>; - void push_back(const T& t [[clang::lifetime_capture_by_this]]) requires IsPointerLikeType<T>; + void push_back(T&& t [[clang::lifetime_capture_by(this)]]) requires IsPointerLikeType<T>; + void push_back(c... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/208691 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
