Re: [PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
On Thu, 2 Aug 2018, Richard Smith via cfe-commits wrote: On Thu, 2 Aug 2018, 06:10 Martin Storsjö via Phabricator via cfe-commits, wrote: mstorsjo added a comment. This change made clang to start trigger failed asserts for me (although they seem to not be reproducible with all compilers building clang), see https://bugs.llvm.org/show_bug.cgi?id=38421 for full description. Are you still seeing problems after r338478? This change is miscompiled by GCC; that revision contains a workaround. Sorry, didn't see this mail until now. Yes, I saw problems even after r338478; I applied the same fix in a different spot as well, in r338749. // Martin ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
On Thu, 2 Aug 2018, 06:10 Martin Storsjö via Phabricator via cfe-commits, < cfe-commits@lists.llvm.org> wrote: > mstorsjo added a comment. > > This change made clang to start trigger failed asserts for me (although > they seem to not be reproducible with all compilers building clang), see > https://bugs.llvm.org/show_bug.cgi?id=38421 for full description. > Are you still seeing problems after r338478? This change is miscompiled by GCC; that revision contains a workaround. Repository: > rC Clang > > https://reviews.llvm.org/D49922 > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
mstorsjo added a comment. This change made clang to start trigger failed asserts for me (although they seem to not be reproducible with all compilers building clang), see https://bugs.llvm.org/show_bug.cgi?id=38421 for full description. Repository: rC Clang https://reviews.llvm.org/D49922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
kimgr added a comment.
Potential typo in the tests
Comment at: test/SemaCXX/attr-lifetimebound.cpp:24
+
+ // Do not diagnose non-void return types; they can still be lifetime-bound.
+ long long ptrintcast(int [[clang::lifetimebound]]) {
Should this say 'non-ref' instead of 'non-void'?
Repository:
rC Clang
https://reviews.llvm.org/D49922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
This revision was automatically updated to reflect the committed changes.
rsmith marked 2 inline comments as done.
Closed by commit rC338464: [P0936R0] add [[clang::lifetimebound]] attribute
(authored by rsmith, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D49922?vs=158149=158430#toc
Repository:
rC Clang
https://reviews.llvm.org/D49922
Files:
include/clang/AST/Type.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Type.cpp
lib/AST/TypePrinter.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaInit.cpp
lib/Sema/SemaType.cpp
test/SemaCXX/attr-lifetimebound.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -3305,11 +3305,16 @@
// Otherwise, generate an appertainsTo check specific to this attribute which
// checks all of the given subjects against the Decl passed in. Return the
// name of that check to the caller.
+ //
+ // If D is null, that means the attribute was not applied to a declaration
+ // at all (for instance because it was applied to a type), or that the caller
+ // has determined that the check should fail (perhaps prior to the creation
+ // of the declaration).
std::string FnName = "check" + Attr.getName().str() + "AppertainsTo";
std::stringstream SS;
SS << "static bool " << FnName << "(Sema , const ParsedAttr , ";
SS << "const Decl *D) {\n";
- SS << " if (";
+ SS << " if (!D || (";
for (auto I = Subjects.begin(), E = Subjects.end(); I != E; ++I) {
// If the subject has custom code associated with it, generate a function
// for it. The function cannot be inlined into this check (yet) because it
@@ -3325,7 +3330,7 @@
if (I + 1 != E)
SS << " && ";
}
- SS << ") {\n";
+ SS << ")) {\n";
SS << "S.Diag(Attr.getLoc(), diag::";
SS << (Warn ? "warn_attribute_wrong_decl_type_str" :
"err_attribute_wrong_decl_type_str");
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -4237,6 +4237,7 @@
attr_null_unspecified,
attr_objc_kindof,
attr_objc_inert_unsafe_unretained,
+attr_lifetimebound,
};
private:
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -141,6 +141,13 @@
isa(S)}],
"non-K functions">;
+// A subject that matches the implicit object parameter of a non-static member
+// function. Accepted as a function type attribute on the type of such a
+// member function.
+// FIXME: This does not actually ever match currently.
+def ImplicitObjectParameter : SubsetSubject;
+
// A single argument to an attribute
class Argument {
string Name = name;
@@ -1211,6 +1218,13 @@
let Documentation = [LayoutVersionDocs];
}
+def LifetimeBound : InheritableAttr {
+ let Spellings = [Clang<"lifetimebound", 0>];
+ let Subjects = SubjectList<[ParmVar, ImplicitObjectParameter], ErrorDiag>;
+ let Documentation = [LifetimeBoundDocs];
+ let LangOpts = [CPlusPlus];
+}
+
def TrivialABI : InheritableAttr {
// This attribute does not have a C [[]] spelling because it requires the
// CPlusPlus language option.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7854,6 +7854,13 @@
"null returned from %select{function|method}0 that requires a non-null return value">,
InGroup;
+def err_lifetimebound_no_object_param : Error<
+ "'lifetimebound' attribute cannot be applied; %select{static |non-}0member "
+ "function has no implicit object parameter">;
+def err_lifetimebound_ctor_dtor : Error<
+ "'lifetimebound' attribute cannot be applied to a "
+ "%select{constructor|destructor}0">;
+
// CHECK: returning address/reference of stack memory
def warn_ret_stack_addr_ref : Warning<
"%select{address of|reference to}0 stack memory associated with "
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -2362,6 +2362,22 @@
}];
}
+def LifetimeBoundDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+The ``lifetimebound`` attribute indicates that a resource owned by
+a function parameter or implicit object parameter
+is retained by the return value of the annotated function
+(or, for a parameter of a constructor, in the value of the constructed object).
+It is only supported in
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
rsmith marked 3 inline comments as done.
rsmith added inline comments.
Comment at: lib/Sema/SemaInit.cpp:6959
+ case IndirectLocalPathEntry::LifetimeBoundCall:
+// FIXME: Consider adding a note for this.
+break;
aaron.ballman wrote:
> Is this something you intended to handle in this patch and forgot, or are you
> saving this for a future patch?
It's something I'm going to experiment with and see if it improves the
diagnostic quality, but not for this patch.
Comment at: test/SemaCXX/attr-lifetimebound.cpp:4
+namespace usage_invalid {
+ // FIXME: Diagnose void return type?
+ void voidreturn(int [[clang::lifetimebound]]);
aaron.ballman wrote:
> Something you intended to support in this patch?
More like an open question for the standards proposal. I've slightly reworded
the FIXME to indicate that this is an open question.
https://reviews.llvm.org/D49922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM, though I raised a few questions. If you want to handle them as part of
this patch, I'm happy to do another round of review. If you want to handle them
in a follow-up patch, I'm also okay.
Comment at: lib/Sema/SemaDecl.cpp:6013
+ // Check the attributes on the function type, if any.
+ if (auto *FD = dyn_cast()) {
+for (TypeLoc TL = FD->getTypeSourceInfo()->getTypeLoc();
rsmith wrote:
> aaron.ballman wrote:
> > `const auto *` (and thread the const-correctness through).
> >
> > Actually, if you could double-check on the const correctness elsewhere in
> > the patch, that'd be useful, as it looks like there are other places that
> > could be similarly touched up.
> Done. I don't think it makes any difference, though: this constness is
> shallow, and doesn't defend against us somehow accidentally mutating the
> `TypeSourceInfo` (which is all we're looking at, and `TypeLoc` doesn't have a
> `const` view).
Yeah, our const-correctness story is pretty hit or miss. However, I try to
ensure new code attempts to be const-correct if it's not too annoying because
I'm still a hopeful, optimistic kind of guy. :-P Thanks for handling this!
Comment at: lib/Sema/SemaDecl.cpp:6022-6028
+if (!MD || MD->isStatic()) {
+ S.Diag(ATL.getAttrNameLoc(), diag::err_lifetimebound_no_object_param)
+ << !MD << ATL.getLocalSourceRange();
+} else if (isa(MD) || isa(MD)) {
+ S.Diag(ATL.getAttrNameLoc(), diag::err_lifetimebound_ctor_dtor)
+ << isa(MD) << ATL.getLocalSourceRange();
+}
rsmith wrote:
> aaron.ballman wrote:
> > Can elide the braces.
> True, but the controlled statements are multi-line, so I'd prefer not to. :)
Sold.
Comment at: lib/Sema/SemaInit.cpp:6959
+ case IndirectLocalPathEntry::LifetimeBoundCall:
+// FIXME: Consider adding a note for this.
+break;
Is this something you intended to handle in this patch and forgot, or are you
saving this for a future patch?
Comment at: lib/Sema/SemaType.cpp:7202-7207
+ if (D.isDeclarationOfFunction()) {
+CurType = S.Context.getAttributedType(AttributedType::attr_lifetimebound,
+ CurType, CurType);
+ } else {
+Attr.diagnoseAppertainsTo(S, nullptr);
+ }
rsmith wrote:
> aaron.ballman wrote:
> > Elide braces
> I'd prefer to not elide braces around a multi-line `if` body, and I'd
> strongly prefer to avoid bracing an `if` and not its `else`.
Sounds good.
Comment at: test/SemaCXX/attr-lifetimebound.cpp:3
+
+namespace usage_invalid {
+ // FIXME: Diagnose void return type?
Can you also add a test demonstrating the attribute doesn't accept any
arguments?
Comment at: test/SemaCXX/attr-lifetimebound.cpp:4
+namespace usage_invalid {
+ // FIXME: Diagnose void return type?
+ void voidreturn(int [[clang::lifetimebound]]);
Something you intended to support in this patch?
Comment at: test/SemaCXX/attr-lifetimebound.cpp:102
+ void use_reversed_range() {
+// FIXME: Don't expose the name of the internal range variable.
+for (auto x : reversed(make_vector())) {} // expected-warning {{temporary
bound to local reference '__range1'}}
Agreed; that is a bit ugly.
https://reviews.llvm.org/D49922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
rsmith added inline comments.
Comment at: include/clang/Basic/AttrDocs.td:2371
+is retained by the return value of the annotated function
+(or, for a constructor, in the value of the constructed object).
+It is only supported in C++.
aaron.ballman wrote:
> I read this as allowing a `lifetimebound` attribute on a constructor, but
> that seems to be an error case.
A `lifetimebound` attribute is permitted on a parameter of a constructor. I've
reworded to clarify.
Comment at: lib/Parse/ParseDeclCXX.cpp:3811
case ParsedAttr::AT_CXX11NoReturn:
+ case ParsedAttr::AT_LifetimeBound:
return true;
aaron.ballman wrote:
> Hmmm, this isn't a standards-based attribute yet. The only thing this
> controls, however, is not parsing a duplicate attribute within the same
> attribute specifier sequence as the standard is the only thing persnickety
> enough to feel that warrants an error.
>
> My personal preference is to not disallow that, especially given that users
> can write `void f() [[clang::lifetimebound]][[clang::lifetimebound]]`
> and get no diagnostic.
This also controls whether redundant parens are permitted, which is what I
wanted to disallow.
However, some testing of this has revealed that I actually do want an optional
argument, so I'll leave the checking out for now, and we can talk about that in
the next patch :)
Comment at: lib/Sema/SemaDecl.cpp:6013
+ // Check the attributes on the function type, if any.
+ if (auto *FD = dyn_cast()) {
+for (TypeLoc TL = FD->getTypeSourceInfo()->getTypeLoc();
aaron.ballman wrote:
> `const auto *` (and thread the const-correctness through).
>
> Actually, if you could double-check on the const correctness elsewhere in the
> patch, that'd be useful, as it looks like there are other places that could
> be similarly touched up.
Done. I don't think it makes any difference, though: this constness is shallow,
and doesn't defend against us somehow accidentally mutating the
`TypeSourceInfo` (which is all we're looking at, and `TypeLoc` doesn't have a
`const` view).
Comment at: lib/Sema/SemaDecl.cpp:6022-6028
+if (!MD || MD->isStatic()) {
+ S.Diag(ATL.getAttrNameLoc(), diag::err_lifetimebound_no_object_param)
+ << !MD << ATL.getLocalSourceRange();
+} else if (isa(MD) || isa(MD)) {
+ S.Diag(ATL.getAttrNameLoc(), diag::err_lifetimebound_ctor_dtor)
+ << isa(MD) << ATL.getLocalSourceRange();
+}
aaron.ballman wrote:
> Can elide the braces.
True, but the controlled statements are multi-line, so I'd prefer not to. :)
Comment at: lib/Sema/SemaOverload.cpp:5233-5235
+ if (From->getType()->isPointerType())
+return From;
+ return TemporaryMaterializationConversion(From);
Reverted this change; I committed this separately.
Comment at: lib/Sema/SemaType.cpp:7202-7207
+ if (D.isDeclarationOfFunction()) {
+CurType = S.Context.getAttributedType(AttributedType::attr_lifetimebound,
+ CurType, CurType);
+ } else {
+Attr.diagnoseAppertainsTo(S, nullptr);
+ }
aaron.ballman wrote:
> Elide braces
I'd prefer to not elide braces around a multi-line `if` body, and I'd strongly
prefer to avoid bracing an `if` and not its `else`.
https://reviews.llvm.org/D49922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
rsmith updated this revision to Diff 158149.
rsmith marked 8 inline comments as done.
https://reviews.llvm.org/D49922
Files:
include/clang/AST/Type.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Type.cpp
lib/AST/TypePrinter.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaInit.cpp
lib/Sema/SemaType.cpp
test/SemaCXX/attr-lifetimebound.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -3305,11 +3305,16 @@
// Otherwise, generate an appertainsTo check specific to this attribute which
// checks all of the given subjects against the Decl passed in. Return the
// name of that check to the caller.
+ //
+ // If D is null, that means the attribute was not applied to a declaration
+ // at all (for instance because it was applied to a type), or that the caller
+ // has determined that the check should fail (perhaps prior to the creation
+ // of the declaration).
std::string FnName = "check" + Attr.getName().str() + "AppertainsTo";
std::stringstream SS;
SS << "static bool " << FnName << "(Sema , const ParsedAttr , ";
SS << "const Decl *D) {\n";
- SS << " if (";
+ SS << " if (!D || (";
for (auto I = Subjects.begin(), E = Subjects.end(); I != E; ++I) {
// If the subject has custom code associated with it, generate a function
// for it. The function cannot be inlined into this check (yet) because it
@@ -3325,7 +3330,7 @@
if (I + 1 != E)
SS << " && ";
}
- SS << ") {\n";
+ SS << ")) {\n";
SS << "S.Diag(Attr.getLoc(), diag::";
SS << (Warn ? "warn_attribute_wrong_decl_type_str" :
"err_attribute_wrong_decl_type_str");
Index: test/SemaCXX/attr-lifetimebound.cpp
===
--- test/SemaCXX/attr-lifetimebound.cpp
+++ test/SemaCXX/attr-lifetimebound.cpp
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -std=c++2a -verify %s
+
+namespace usage_invalid {
+ // FIXME: Diagnose void return type?
+ void voidreturn(int [[clang::lifetimebound]]);
+
+ int *not_class_member() [[clang::lifetimebound]]; // expected-error {{non-member function has no implicit object parameter}}
+ struct A {
+A() [[clang::lifetimebound]]; // expected-error {{cannot be applied to a constructor}}
+~A() [[clang::lifetimebound]]; // expected-error {{cannot be applied to a destructor}}
+static int *static_class_member() [[clang::lifetimebound]]; // expected-error {{static member function has no implicit object parameter}}
+int not_function [[clang::lifetimebound]]; // expected-error {{only applies to parameters and implicit object parameters}}
+int [[clang::lifetimebound]] also_not_function; // expected-error {{cannot be applied to types}}
+ };
+}
+
+namespace usage_ok {
+ struct IntRef { int *target; };
+
+ int (int [[clang::lifetimebound]]);
+ int (IntRef param [[clang::lifetimebound]]);
+
+ // Do not diagnose non-void return types; they can still be lifetime-bound.
+ long long ptrintcast(int [[clang::lifetimebound]]) {
+return (long long)
+ }
+ // Likewise.
+ int (long long param [[clang::lifetimebound]]) {
+return *(int*)param;
+ }
+
+ struct A {
+A();
+A(int);
+int *class_member() [[clang::lifetimebound]];
+operator int*() [[clang::lifetimebound]];
+ };
+
+ int *p = A().class_member(); // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
+ int *q = A(); // expected-warning {{temporary whose address is used as value of local variable 'q' will be destroyed at the end of the full-expression}}
+ int *r = A(1); // expected-warning {{temporary whose address is used as value of local variable 'r' will be destroyed at the end of the full-expression}}
+}
+
+# 1 "" 1 3
+namespace std {
+ using size_t = __SIZE_TYPE__;
+ struct string {
+string();
+string(const char*);
+
+char [](size_t) const [[clang::lifetimebound]];
+ };
+ string operator""s(const char *, size_t);
+
+ struct string_view {
+string_view();
+string_view(const char *p [[clang::lifetimebound]]);
+string_view(const string [[clang::lifetimebound]]);
+ };
+ string_view operator""sv(const char *, size_t);
+
+ struct vector {
+int *data();
+size_t size();
+ };
+
+ template struct map {};
+}
+# 68 "attr-lifetimebound.cpp" 2
+
+using std::operator""s;
+using std::operator""sv;
+
+namespace p0936r0_examples {
+ std::string_view s = "foo"s; // expected-warning {{temporary}}
+
+ std::string operator+(std::string_view s1, std::string_view s2);
+ void f() {
+std::string_view sv = "hi";
+std::string_view sv2 = sv + sv; // expected-warning {{temporary}}
+sv2 = sv + sv; // FIXME: can we infer that we
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
aaron.ballman added inline comments.
Comment at: include/clang/Basic/AttrDocs.td:2371
+is retained by the return value of the annotated function
+(or, for a constructor, in the value of the constructed object).
+It is only supported in C++.
I read this as allowing a `lifetimebound` attribute on a constructor, but that
seems to be an error case.
Comment at: lib/AST/TypePrinter.cpp:1492-1521
- case AttributedType::attr_objc_gc: {
-OS << "objc_gc(";
-
-QualType tmp = T->getEquivalentType();
-while (tmp.getObjCGCAttr() == Qualifiers::GCNone) {
- QualType next = tmp->getPointeeType();
- if (next == tmp) break;
rsmith wrote:
> (This was dead code; see lines 1396-1399.)
Good catch! Do you mind committing this separately?
Comment at: lib/Parse/ParseDeclCXX.cpp:3811
case ParsedAttr::AT_CXX11NoReturn:
+ case ParsedAttr::AT_LifetimeBound:
return true;
Hmmm, this isn't a standards-based attribute yet. The only thing this controls,
however, is not parsing a duplicate attribute within the same attribute
specifier sequence as the standard is the only thing persnickety enough to feel
that warrants an error.
My personal preference is to not disallow that, especially given that users can
write `void f() [[clang::lifetimebound]][[clang::lifetimebound]]` and get
no diagnostic.
Comment at: lib/Sema/SemaDecl.cpp:6013
+ // Check the attributes on the function type, if any.
+ if (auto *FD = dyn_cast()) {
+for (TypeLoc TL = FD->getTypeSourceInfo()->getTypeLoc();
`const auto *` (and thread the const-correctness through).
Actually, if you could double-check on the const correctness elsewhere in the
patch, that'd be useful, as it looks like there are other places that could be
similarly touched up.
Comment at: lib/Sema/SemaDecl.cpp:6018
+ // The [[lifetimebound]] attribute can be applied to the implicit object
+ // parameter of a non-static member function (other than a dtor) by
+ // applying it to the function type.
This comment doesn't match reality regarding ctors.
Comment at: lib/Sema/SemaDecl.cpp:6022-6028
+if (!MD || MD->isStatic()) {
+ S.Diag(ATL.getAttrNameLoc(), diag::err_lifetimebound_no_object_param)
+ << !MD << ATL.getLocalSourceRange();
+} else if (isa(MD) || isa(MD)) {
+ S.Diag(ATL.getAttrNameLoc(), diag::err_lifetimebound_ctor_dtor)
+ << isa(MD) << ATL.getLocalSourceRange();
+}
Can elide the braces.
Comment at: lib/Sema/SemaInit.cpp:6369
+
+static bool implicitObjectParamIsLifetimeBound(FunctionDecl *FD) {
+ TypeSourceInfo *TSI = FD->getTypeSourceInfo();
`const FunctionDecl *` (and threaded through).
Comment at: lib/Sema/SemaType.cpp:7202-7207
+ if (D.isDeclarationOfFunction()) {
+CurType = S.Context.getAttributedType(AttributedType::attr_lifetimebound,
+ CurType, CurType);
+ } else {
+Attr.diagnoseAppertainsTo(S, nullptr);
+ }
Elide braces
Repository:
rC Clang
https://reviews.llvm.org/D49922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
rsmith added inline comments.
Comment at: include/clang/AST/TypeLoc.h:96-97
/// Convert to the specified TypeLoc type, returning a null TypeLoc if
- /// this TypeLock is not of the desired type. It will consider type
- /// adjustments from a type that wad written as a T to another type that is
+ /// this TypeLoc is not of the desired type. It will consider type
+ /// adjustments from a type that was written as a T to another type that is
/// still canonically a T (ignores parens, attributes, elaborated types,
etc).
(This typo fix has been committed separately.)
Comment at: include/clang/Basic/AttrDocs.td:2368
+ let Content = [{
+The ``lifetime_bound`` attribute indicates that a resource owned by
+a function parameter or implicit object parameter
I've removed the underscore here.
Comment at: lib/AST/TypePrinter.cpp:1492-1521
- case AttributedType::attr_objc_gc: {
-OS << "objc_gc(";
-
-QualType tmp = T->getEquivalentType();
-while (tmp.getObjCGCAttr() == Qualifiers::GCNone) {
- QualType next = tmp->getPointeeType();
- if (next == tmp) break;
(This was dead code; see lines 1396-1399.)
Repository:
rC Clang
https://reviews.llvm.org/D49922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49922: [P0936R0] add [[clang::lifetimebound]] attribute
rsmith created this revision.
rsmith added a reviewer: aaron.ballman.
Herald added a reviewer: javed.absar.
Herald added a subscriber: kristof.beyls.
This patch adds support for a new attribute, [[clang::lifetimebound]], that
indicates that the lifetime of a function result is related to one of the
function arguments. When walking an initializer to make sure that the lifetime
of the initial value is at least as long as the lifetime of the initialized
object, we step through parameters (including the implicit object parameter of
a non-static member function) that are marked with this attribute.
There's nowhere to write an attribute on the implicit object parameter, so in
lieu of that, it may be applied to a function type (where it appears
immediately after the cv-qualifiers and ref-qualifier, which is as close to a
declaration of the implicit object parameter as we have). I'm currently
modeling this in the AST as the attribute appertaining to a `ParmVarDecl` or to
a function type, but in the latter case I'd be happy to stash it somewhere else
if there is a better home for it.
Repository:
rC Clang
https://reviews.llvm.org/D49922
Files:
include/clang/AST/Type.h
include/clang/AST/TypeLoc.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Type.cpp
lib/AST/TypePrinter.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaInit.cpp
lib/Sema/SemaOverload.cpp
lib/Sema/SemaType.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -3305,11 +3305,16 @@
// Otherwise, generate an appertainsTo check specific to this attribute which
// checks all of the given subjects against the Decl passed in. Return the
// name of that check to the caller.
+ //
+ // If D is null, that means the attribute was not applied to a declaration
+ // at all (for instance because it was applied to a type), or that the caller
+ // has determined that the check should fail (perhaps prior to the creation
+ // of the declaration).
std::string FnName = "check" + Attr.getName().str() + "AppertainsTo";
std::stringstream SS;
SS << "static bool " << FnName << "(Sema , const ParsedAttr , ";
SS << "const Decl *D) {\n";
- SS << " if (";
+ SS << " if (!D || (";
for (auto I = Subjects.begin(), E = Subjects.end(); I != E; ++I) {
// If the subject has custom code associated with it, generate a function
// for it. The function cannot be inlined into this check (yet) because it
@@ -3325,7 +3330,7 @@
if (I + 1 != E)
SS << " && ";
}
- SS << ") {\n";
+ SS << ")) {\n";
SS << "S.Diag(Attr.getLoc(), diag::";
SS << (Warn ? "warn_attribute_wrong_decl_type_str" :
"err_attribute_wrong_decl_type_str");
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5233,6 +5233,8 @@
return ParsedAttr::AT_ObjCKindOf;
case AttributedType::attr_ns_returns_retained:
return ParsedAttr::AT_NSReturnsRetained;
+ case AttributedType::attr_lifetimebound:
+return ParsedAttr::AT_LifetimeBound;
}
llvm_unreachable("unexpected attribute kind!");
}
@@ -7194,6 +7196,18 @@
T = State.getSema().Context.getAddrSpaceQualType(T, ImpAddr);
}
+static void HandleLifetimeBoundAttr(QualType ,
+const ParsedAttr ,
+Sema , Declarator ) {
+ if (D.isDeclarationOfFunction()) {
+CurType = S.Context.getAttributedType(AttributedType::attr_lifetimebound,
+ CurType, CurType);
+ } else {
+Attr.diagnoseAppertainsTo(S, nullptr);
+ }
+}
+
+
static void processTypeAttrs(TypeProcessingState , QualType ,
TypeAttrLocation TAL,
ParsedAttributesView ) {
@@ -7298,6 +7312,13 @@
HandleOpenCLAccessAttr(type, attr, state.getSema());
attr.setUsedAsTypeAttr();
break;
+case ParsedAttr::AT_LifetimeBound:
+ if (TAL == TAL_DeclChunk) {
+HandleLifetimeBoundAttr(type, attr, state.getSema(),
+state.getDeclarator());
+attr.setUsedAsTypeAttr();
+ }
+ break;
MS_TYPE_ATTRS_CASELIST:
if (!handleMSPointerTypeQualifierAttr(state, attr, type))
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5230,7 +5230,9 @@
if (!Context.hasSameType(From->getType(), DestType))
From = ImpCastExprToType(From, DestType, CK_NoOp,
From->getValueKind()).get();
- return From;
+ if
