[cfe-dev] [PATCH] Add an attribute to forbid temporary instances of a type (issue3745045)

jyasskin at gmail.com jyasskin at gmail.com
Tue Dec 28 18:00:32 PST 2010


Reviewers: ,

Message:
This is the first attribute I've added, so please let me know about
anything I could have done better.

Current diff at
http://codereview.appspot.com/download/issue3745045_1.diff

Description:
This allows class authors to write "class
__attribute__((no_temporaries)) Name ..." when they want to force users
to name all variables of the type. This protects people from doing
things like creating a scoped_lock that only lives for a single
statement instead of an entire scope.

Please review this at http://codereview.appspot.com/3745045/

Affected files:
   M     docs/LanguageExtensions.html
   include/clang/Basic/Attr.td
   M     include/clang/Basic/DiagnosticSemaKinds.td
   M     include/clang/Sema/AttributeList.h
   lib/Sema/AttributeList.cpp
   lib/Sema/SemaDeclAttr.cpp
   lib/Sema/SemaExprCXX.cpp
   A     test/SemaCXX/no-temporaries.cpp


Index: test/SemaCXX/no-temporaries.cpp
===================================================================
--- test/SemaCXX/no-temporaries.cpp	(revision 0)
+++ test/SemaCXX/no-temporaries.cpp	(revision 0)
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#if !__has_attribute(no_temporaries)
+#error "Should support no_temporaries attribute"
+#endif
+
+class __attribute__((no_temporaries)) NotATemporary {
+};
+
+class __attribute__((no_temporaries(1))) ShouldntHaveArguments {  //  
expected-error {{attribute requires 0 argument(s)}}
+};
+
+void bad_function() __attribute__((no_temporaries));  // expected-warning  
{{'no_temporaries' attribute only applies to class types}}
+
+int var __attribute__((no_temporaries));  // expected-warning  
{{'no_temporaries' attribute only applies to class types}}
+
+void bar(const NotATemporary&);
+
+void foo() {
+  NotATemporary this_is_fine;
+  bar(NotATemporary());  // expected-error {{Must not create temporaries  
of type 'NotATemporary'}}
+  NotATemporary();   // expected-error {{Must not create temporaries of  
type 'NotATemporary'}}
+}
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td	(revision 122515)
+++ include/clang/Basic/Attr.td	(working copy)
@@ -328,6 +328,11 @@
    let Subjects = [Function];
  }

+def NoTemporaries : Attr {
+  let Spellings = ["no_temporaries"];
+  let Subjects = [CXXRecord];
+}
+
  def NoThrow : Attr {
    let Spellings = ["nothrow"];
  }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td	(revision 122515)
+++ include/clang/Basic/DiagnosticSemaKinds.td	(working copy)
@@ -839,6 +839,8 @@
    "of type %1 invokes deleted constructor">;
  def err_temp_copy_incomplete : Error<
    "copying a temporary object of incomplete type %0">;
+def err_no_temporaries_allowed : Error<
+  "Must not create temporaries of type %0">;

  // C++0x decltype
  def err_cannot_determine_declared_type_of_overloaded_function : Error<
Index: include/clang/Sema/AttributeList.h
===================================================================
--- include/clang/Sema/AttributeList.h	(revision 122515)
+++ include/clang/Sema/AttributeList.h	(working copy)
@@ -119,6 +119,7 @@
      AT_nodebug,
      AT_noinline,
      AT_no_instrument_function,
+    AT_no_temporaries,
      AT_nocommon,
      AT_nonnull,
      AT_noreturn,
Index: docs/LanguageExtensions.html
===================================================================
--- docs/LanguageExtensions.html	(revision 122515)
+++ docs/LanguageExtensions.html	(working copy)
@@ -25,6 +25,7 @@
  <li><a href="#vectors">Vectors and Extended Vectors</a></li>
  <li><a href="#deprecated">Messages on <tt>deprecated</tt> and  
<tt>unavailable</tt> attributes</a></li>
  <li><a href="#attributes-on-enumerators">Attributes on enumerators</a></li>
+<li><a href="#no-temporaries-attribute">Attribute to forbid temporaries of  
a type</a></li>
  <li><a href="#checking_language_features">Checks for Standard Language  
Features</a></li>
    <ul>
    <li><a href="#cxx_exceptions">C++ exceptions</a></li>
@@ -342,6 +343,32 @@
  <p>Query for this feature with  
<tt>__has_feature(enumerator_attributes)</tt>.</p>

  <!--  
======================================================================= -->
+<h2 id="no-temporaries-attribute">Attribute to forbid temporaries of a  
type</h2>
+<!--  
======================================================================= -->
+
+<p>Clang provides a <tt>no_temporaries</tt> attribute to forbid
+temporaries of a particular type.</p>
+
+<blockquote>
+<pre>class __attribute__((no_temporaries)) scoped_lock {
+  ...
+};</pre>
+</blockquote>
+
+<p>This prevents mistakes like</p>
+
+<blockquote>
+<pre>void foo() {
+  scoped_lock(my_mutex);
+  // Forgot the local variable name, so destructor runs here.
+  code_that_needs_lock_held();
+  // User expects destructor to run here.
+};</pre>
+</blockquote>
+
+<p>Query for this feature with  
<tt>__has_attribute(no_temporaries)</tt>.</p>
+
+<!--  
======================================================================= -->
  <h2 id="checking_language_features">Checks for Standard Language  
Features</h2>
  <!--  
======================================================================= -->

Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp	(revision 122515)
+++ lib/Sema/SemaExprCXX.cpp	(working copy)
@@ -3179,9 +3179,12 @@
      }
    }

+  CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
+  if (RD->getAttr<NoTemporariesAttr>())
+    Diag(E->getExprLoc(), diag::err_no_temporaries_allowed) <<  
E->getType();
+
    // That should be enough to guarantee that this type is complete.
    // If it has a trivial destructor, we can avoid the extra copy.
-  CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
    if (RD->isInvalidDecl() || RD->hasTrivialDestructor())
      return Owned(E);

Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp	(revision 122515)
+++ lib/Sema/SemaDeclAttr.cpp	(working copy)
@@ -860,6 +860,24 @@
    d->addAttr(::new (S.Context) VecReturnAttr(Attr.getLoc(), S.Context));
  }

+static void HandleNoTemporariesAttr(Decl *d, const AttributeList &Attr,
+                                    Sema &S) {
+  assert(Attr.isInvalid() == false);
+
+  if (Attr.getNumArgs() != 0) {
+    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << 0;
+    return;
+  }
+
+  if (!isa<TypeDecl>(d)) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
+      << Attr.getName() << 9 /*class*/;
+    return;
+  }
+
+  d->addAttr(::new (S.Context) NoTemporariesAttr(Attr.getLoc(),  
S.Context));
+}
+
  static void HandleDependencyAttr(Decl *d, const AttributeList &Attr, Sema  
&S) {
    if (!isFunctionOrMethod(d) && !isa<ParmVarDecl>(d)) {
      S.Diag(Attr.getLoc(), diag::err_attribute_wrong_decl_type)
@@ -2648,6 +2666,8 @@
        HandleOwnershipAttr     (D, Attr, S); break;
    case AttributeList::AT_naked:       HandleNakedAttr       (D, Attr, S);  
break;
    case AttributeList::AT_noreturn:    HandleNoReturnAttr    (D, Attr, S);  
break;
+  case AttributeList::AT_no_temporaries:
+    HandleNoTemporariesAttr(D, Attr, S); break;
    case AttributeList::AT_nothrow:     HandleNothrowAttr     (D, Attr, S);  
break;
    case AttributeList::AT_override:    HandleOverrideAttr    (D, Attr, S);  
break;
    case AttributeList::AT_shared:      HandleSharedAttr      (D, Attr, S);  
break;
Index: lib/Sema/AttributeList.cpp
===================================================================
--- lib/Sema/AttributeList.cpp	(revision 122515)
+++ lib/Sema/AttributeList.cpp	(working copy)
@@ -118,6 +118,7 @@
      .Case("reqd_work_group_size", AT_reqd_wg_size)
      .Case("init_priority", AT_init_priority)
      .Case("no_instrument_function", AT_no_instrument_function)
+    .Case("no_temporaries", AT_no_temporaries)
      .Case("thiscall", AT_thiscall)
      .Case("pascal", AT_pascal)
      .Case("__cdecl", AT_cdecl)





More information about the cfe-dev mailing list