CONSUMABLE class attribute

[dblaikie]

Summary

Adds the 'consumable' attribute that can be attached to classes. This replaces the previous method of scanning a class's methods to see if any of them have consumed analysis attributes attached to them. If consumed analysis attributes are attached to methods of a class that isn't marked 'consumable' a warning is generated.

[delesley]

lib/Sema/SemaDeclAttr.cpp

981

This block of code seems to have been cut-and-pasted 4 times. Can you factor it into a separate function?

[chriswailes]

Extracted common code out into a function.

[delesley]

lib/Sema/SemaDeclAttr.cpp

1002

Most of the check is still duplicated. I was thinking of:

if (!checkThisTypeIsConsumable(S, D))

return;

[chriswailes]

Factored out even more code.

[delesley]

LGTM.

[aaron.ballman]

Why does this no longer apply to CXXConstructors when it used to previously?

Aside from not understanding that, LGTM!

~Aaron

[delesley]

Submitted as r189702.

[dblaikie]

Already committed in r189702

include/clang/Analysis/Analyses/Consumed.h

   /// A class that handles the analysis of uniqueness violations.
   class ConsumedAnalyzer {
     
-    typedef llvm::DenseMap<const CXXRecordDecl *, bool> CacheMapType;
-    typedef std::pair<const CXXRecordDecl *, bool> CachePairType;
-    
     ConsumedBlockInfo BlockInfo;
     ConsumedStateMap *CurrStates;
     
-    CacheMapType ConsumableTypeCache;
-    
     bool hasConsumableAttributes(const CXXRecordDecl *RD);
     bool splitState(const CFGBlock *CurrBlock,
                     const ConsumedStmtVisitor &Visitor);

     ConsumedAnalyzer(ConsumedWarningsHandlerBase &WarningsHandler)
         : WarningsHandler(WarningsHandler) {}
 
-    /// \brief Check to see if the type is a consumable type.
-    bool isConsumableType(QualType Type);
-    
     /// \brief Check a function's CFG for consumed violations.
     ///
     /// We traverse the blocks in the CFG, keeping track of the state of each

include/clang/Basic/Attr.td

 
 // C/C++ consumed attributes.
 
+def Consumable : InheritableAttr {
+  let Spellings = [GNU<"consumable">];
+  let Subjects = [CXXRecord];
+}
+
 def CallableWhenUnconsumed : InheritableAttr {
   let Spellings = [GNU<"callable_when_unconsumed">];
   let Subjects = [CXXMethod];

   let Subjects = [CXXMethod];
 }
 
-def Consumes : InheritableAttr {
-  let Spellings = [GNU<"consumes">];
-  let Subjects = [CXXMethod, CXXConstructor];
-}
-
 def TestsConsumed : InheritableAttr {
   let Spellings = [GNU<"tests_consumed">];
   let Subjects = [CXXMethod];
 }
 
+def Consumes : InheritableAttr {
+  let Spellings = [GNU<"consumes">];
+  let Subjects = [CXXMethod];
+}
+
 // Type safety attributes for `void *' pointers and type tags.
 
 def ArgumentWithTypeTag : InheritableAttr {

include/clang/Basic/DiagnosticSemaKinds.td

 def warn_use_of_temp_while_consumed : Warning<
   "invocation of method '%0' on a temporary object while it is in the "
   "'consumed' state">, InGroup<Consumed>, DefaultIgnore;
+def warn_attr_on_unconsumable_class : Warning<
+  "consumed analysis attribute is attached to class '%0' which isn't marked "
+  "as consumable">, InGroup<Consumed>, DefaultIgnore;
 
 // ConsumedStrict warnings
 def warn_use_in_unknown_state : Warning<

lib/Analysis/Consumed.cpp

   llvm_unreachable("invalid enum");
 }
 
+static bool isConsumableType(const QualType &QT) {
+  if (const CXXRecordDecl *RD = QT->getAsCXXRecordDecl())
+    return RD->hasAttr<ConsumableAttr>();
+  else
+    return false;
+}
+
 static bool isKnownState(ConsumedState State) {
   switch (State) {
   case CS_Unconsumed:

   ASTContext &CurrContext = AC.getASTContext();
   QualType ThisType = Constructor->getThisType(CurrContext)->getPointeeType();
   
-  if (Analyzer.isConsumableType(ThisType)) {
+  if (isConsumableType(ThisType)) {
     if (Constructor->hasAttr<ConsumesAttr>() ||
         Constructor->isDefaultConstructor()) {
       

 
 
 void ConsumedStmtVisitor::VisitParmVarDecl(const ParmVarDecl *Param) {
-  if (Analyzer.isConsumableType(Param->getType()))
+  if (isConsumableType(Param->getType()))
     StateMap->setState(Param, consumed::CS_Unknown);
 }
 

 }
 
 void ConsumedStmtVisitor::VisitVarDecl(const VarDecl *Var) {
-  if (Analyzer.isConsumableType(Var->getType())) {
+  if (isConsumableType(Var->getType())) {
     if (Var->hasInit()) {
       PropagationInfo PInfo =
         PropagationMap.find(Var->getInit())->second;

   Map.erase(Var);
 }
 
-bool ConsumedAnalyzer::isConsumableType(QualType Type) {
-  const CXXRecordDecl *RD =
-    dyn_cast_or_null<CXXRecordDecl>(Type->getAsCXXRecordDecl());
-  
-  if (!RD) return false;
-  
-  std::pair<CacheMapType::iterator, bool> Entry =
-    ConsumableTypeCache.insert(std::make_pair(RD, false));
-  
-  if (Entry.second)
-    Entry.first->second = hasConsumableAttributes(RD);
-  
-  return Entry.first->second;
-}
-
-// TODO: Walk the base classes to see if any of them are unique types.
-//       (Deferred)
-bool ConsumedAnalyzer::hasConsumableAttributes(const CXXRecordDecl *RD) {
-  for (CXXRecordDecl::method_iterator MI = RD->method_begin(),
-       ME = RD->method_end(); MI != ME; ++MI) {
-    
-    for (Decl::attr_iterator AI = (*MI)->attr_begin(), AE = (*MI)->attr_end();
-         AI != AE; ++AI) {
-      
-      switch ((*AI)->getKind()) {
-      case attr::CallableWhenUnconsumed:
-      case attr::TestsUnconsumed:
-        return true;
-      
-      default:
-        break;
-      }
-    }
-  }
-  
-  return false;
-}
-
 bool ConsumedAnalyzer::splitState(const CFGBlock *CurrBlock,
                                   const ConsumedStmtVisitor &Visitor) {
   

lib/Sema/SemaDeclAttr.cpp

                                Attr.getAttributeSpellingListIndex()));
 }
 
-static void handleConsumesAttr(Sema &S, Decl *D,
-                               const AttributeList &Attr) {
+static void handleConsumableAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   if (!checkAttributeNumArgs(S, Attr, 0)) return;
 
-  if (!(isa<CXXMethodDecl>(D) || isa<CXXConstructorDecl>(D))) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << Attr.getName() << ExpectedMethod;
+  if (!isa<CXXRecordDecl>(D)) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+      Attr.getName() << ExpectedClass;
     return;
   }
   
   D->addAttr(::new (S.Context)
+             ConsumableAttr(Attr.getRange(), S.Context,
+                            Attr.getAttributeSpellingListIndex()));
+}
+
+static bool checkForConsumableClass(Sema &S, const CXXMethodDecl *MD,
+                                        const AttributeList &Attr) {
+  ASTContext &CurrContext = S.getASTContext();
+  QualType ThisType = MD->getThisType(CurrContext)->getPointeeType();
+  
+  if (const CXXRecordDecl *RD = ThisType->getAsCXXRecordDecl()) {
+    if (!RD->hasAttr<ConsumableAttr>()) {
+      S.Diag(Attr.getLoc(), diag::warn_attr_on_unconsumable_class) <<
+        RD->getNameAsString();
+      
+      return false;
+    }
+  }
+  
+  return true;
+}
+
+static void handleConsumesAttr(Sema &S, Decl *D, const AttributeList &Attr) {
+  if (!checkAttributeNumArgs(S, Attr, 0)) return;
+
+  if (!isa<CXXMethodDecl>(D)) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+      Attr.getName() << ExpectedMethod;
+    return;
+  }
+  
+  if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
+    return;
+  
+  D->addAttr(::new (S.Context)
              ConsumesAttr(Attr.getRange(), S.Context,
                           Attr.getAttributeSpellingListIndex()));
 }
 
 static void handleCallableWhenUnconsumedAttr(Sema &S, Decl *D,

   if (!checkAttributeNumArgs(S, Attr, 0)) return;
 
   if (!isa<CXXMethodDecl>(D)) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << Attr.getName() << ExpectedMethod;
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+      Attr.getName() << ExpectedMethod;
     return;
   }
   
+  if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
+    return;
+  
   D->addAttr(::new (S.Context)
              CallableWhenUnconsumedAttr(Attr.getRange(), S.Context,
                                         Attr.getAttributeSpellingListIndex()));
 }
 
 static void handleTestsConsumedAttr(Sema &S, Decl *D,

   if (!checkAttributeNumArgs(S, Attr, 0)) return;
 
   if (!isa<CXXMethodDecl>(D)) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << Attr.getName() << ExpectedMethod;
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+      Attr.getName() << ExpectedMethod;
     return;
   }
   
+  if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
+    return;
+  
   D->addAttr(::new (S.Context)
              TestsConsumedAttr(Attr.getRange(), S.Context,
                                Attr.getAttributeSpellingListIndex()));
 }
 
 static void handleTestsUnconsumedAttr(Sema &S, Decl *D,

   if (!checkAttributeNumArgs(S, Attr, 0)) return;
 
   if (!isa<CXXMethodDecl>(D)) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << Attr.getName() << ExpectedMethod;
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+      Attr.getName() << ExpectedMethod;
     return;
   }
   
+  if (!checkForConsumableClass(S, cast<CXXMethodDecl>(D), Attr))
+    return;
+  
   D->addAttr(::new (S.Context)
              TestsUnconsumedAttr(Attr.getRange(), S.Context,
                                  Attr.getAttributeSpellingListIndex()));
 }
 
 static void handleExtVectorTypeAttr(Sema &S, Scope *scope, Decl *D,

     break;
 
   // Uniqueness analysis attributes.
+  case AttributeList::AT_Consumable:
+    handleConsumableAttr(S, D, Attr);
+    break;
   case AttributeList::AT_Consumes:
     handleConsumesAttr(S, D, Attr);
     break;

test/SemaCXX/warn-consumed-analysis-strict.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed-strict -std=c++11 %s
 
 #define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed))
-#define CONSUMES __attribute__ ((consumes))
-#define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed))
+#define CONSUMABLE               __attribute__ ((consumable))
+#define CONSUMES                 __attribute__ ((consumes))
+#define TESTS_UNCONSUMED         __attribute__ ((tests_unconsumed))
 
 #define TEST_VAR(Var) Var.isValid()
 
 typedef decltype(nullptr) nullptr_t;
 
 template <typename T>
-class ConsumableClass {
+class CONSUMABLE ConsumableClass {
   T var;
   
   public:

test/SemaCXX/warn-consumed-analysis.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s
 
 #define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed))
-#define CONSUMES __attribute__ ((consumes))
-#define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed))
+#define CONSUMABLE               __attribute__ ((consumable))
+#define CONSUMES                 __attribute__ ((consumes))
+#define TESTS_UNCONSUMED         __attribute__ ((tests_unconsumed))
 
 typedef decltype(nullptr) nullptr_t;
 
 template <typename T>
-class ConsumableClass {
+class CONSUMABLE ConsumableClass {
   T var;
   
   public:

test/SemaCXX/warn-consumed-parsing.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s
 
+#define CONSUMABLE                __attribute__ ((consumable))
 #define CONSUMES                  __attribute__ ((consumes))
 #define TESTS_UNCONSUMED          __attribute__ ((tests_unconsumed))
 #define CALLABLE_WHEN_UNCONSUMED  __attribute__ ((callable_when_unconsumed))
 
 class AttrTester0 {
-  void Consumes(void)        __attribute__ ((consumes(42))); // expected-error {{attribute takes no arguments}}
-  bool TestsUnconsumed(void) __attribute__ ((tests_unconsumed(42))); // expected-error {{attribute takes no arguments}}
-  void CallableWhenUnconsumed(void) 
+  void Consumes()        __attribute__ ((consumes(42))); // expected-error {{attribute takes no arguments}}
+  bool TestsUnconsumed() __attribute__ ((tests_unconsumed(42))); // expected-error {{attribute takes no arguments}}
+  void CallableWhenUnconsumed() 
     __attribute__ ((callable_when_unconsumed(42))); // expected-error {{attribute takes no arguments}}
 };
 
 int var0 CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}}
 int var1 TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' attribute only applies to methods}}
 int var2 CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' attribute only applies to methods}}
+int var3 CONSUMABLE; // expected-warning {{'consumable' attribute only applies to classes}}
 
-void function0(void) CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}}
-void function1(void) TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' attribute only applies to methods}}
-void function2(void) CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' attribute only applies to methods}}
-
-class AttrTester1 {
-  void consumes(void)        CONSUMES;
-  bool testsUnconsumed(void) TESTS_UNCONSUMED;
+void function0() CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}}
+void function1() TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' attribute only applies to methods}}
+void function2() CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' attribute only applies to methods}}
+void function3() CONSUMABLE; // expected-warning {{'consumable' attribute only applies to classes}}
+
+class CONSUMABLE AttrTester1 {
+  void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED;
+  void consumes()               CONSUMES;
+  bool testsUnconsumed()        TESTS_UNCONSUMED;
 };
 
 class AttrTester2 {
-  void callableWhenUnconsumed(void) CALLABLE_WHEN_UNCONSUMED;
+  void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to class 'AttrTester2' which isn't marked as consumable}}
+  void consumes()               CONSUMES; // expected-warning {{consumed analysis attribute is attached to class 'AttrTester2' which isn't marked as consumable}}
+  bool testsUnconsumed()        TESTS_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to class 'AttrTester2' which isn't marked as consumable}}
 };