Thread safety analysis: split warnings into two groups: attribute warnings

which are checked in the parser, and analysis warnings that require the
full analysis.  This allows attribute syntax to be checked independently
of the full thread safety analysis.  Also introduces a new warning for the
case where a string is used as a lock expression; this allows the analysis
to gracefully handle expressions that would otherwise cause a parse error.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@155129 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 46 insertions, 48 deletions

diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index 5c6ddd2cb9..3770b3c373 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -243,12 +243,13 @@ static bool isIntOrBool(Expr *Exp) {
 /// Note that this function may produce an error message.
 /// \return true if the Decl is a pointer type; false otherwise
 ///
-static bool checkIsPointer(Sema &S, const Decl *D, const AttributeList &Attr) {
+static bool threadSafetyCheckIsPointer(Sema &S, const Decl *D,
+                                       const AttributeList &Attr) {
   if (const ValueDecl *vd = dyn_cast<ValueDecl>(D)) {
     QualType QT = vd->getType();
     if (QT->isAnyPointerType())
       return true;
-    S.Diag(Attr.getLoc(), diag::warn_pointer_attribute_wrong_type)
+    S.Diag(Attr.getLoc(), diag::warn_thread_attribute_decl_not_pointer)
       << Attr.getName()->getName() << QT;
   } else {
     S.Diag(Attr.getLoc(), diag::err_attribute_can_be_applied_only_to_value_decl)
@@ -271,14 +272,14 @@ static const RecordType *getRecordType(QualType QT) {
 }
 
 /// \brief Thread Safety Analysis: Checks that the passed in RecordType
-/// resolves to a lockable object. May flag an error.
+/// resolves to a lockable object.
 static void checkForLockableRecord(Sema &S, Decl *D, const AttributeList &Attr,
                                    QualType Ty) {
   const RecordType *RT = getRecordType(Ty);
                                    
   // Warn if could not get record type for this argument.
   if (!RT) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_argument_not_class)
+    S.Diag(Attr.getLoc(), diag::warn_thread_attribute_argument_not_class)
       << Attr.getName() << Ty.getAsString();
     return;
   }
@@ -287,18 +288,18 @@ static void checkForLockableRecord(Sema &S, Decl *D, const AttributeList &Attr,
     return;
   // Warn if the type is not lockable.
   if (!RT->getDecl()->getAttr<LockableAttr>()) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_argument_not_lockable)
+    S.Diag(Attr.getLoc(), diag::warn_thread_attribute_argument_not_lockable)
       << Attr.getName() << Ty.getAsString();
     return;
   }
 }
 
 /// \brief Thread Safety Analysis: Checks that all attribute arguments, starting
-/// from Sidx, resolve to a lockable object. May flag an error.
+/// from Sidx, resolve to a lockable object.
 /// \param Sidx The attribute argument index to start checking with.
 /// \param ParamIdxOk Whether an argument can be indexing into a function
 /// parameter list.
-static bool checkAttrArgsAreLockableObjs(Sema &S, Decl *D,
+static void checkAttrArgsAreLockableObjs(Sema &S, Decl *D,
                                          const AttributeList &Attr,
                                          SmallVectorImpl<Expr*> &Args,
                                          int Sidx = 0,
@@ -307,11 +308,19 @@ static bool checkAttrArgsAreLockableObjs(Sema &S, Decl *D,
     Expr *ArgExp = Attr.getArg(Idx);
 
     if (ArgExp->isTypeDependent()) {
-      // FIXME -- need to processs this again on template instantiation
+      // FIXME -- need to check this again on template instantiation
       Args.push_back(ArgExp);
       continue;
     }
 
+    if (isa<StringLiteral>(ArgExp)) {
+      // We allow constant strings to be used as a placeholder for expressions
+      // that are not valid C++ syntax, but warn that they are ignored.
+      S.Diag(Attr.getLoc(), diag::warn_thread_attribute_ignored) <<
+        Attr.getName();
+      continue;
+    }
+
     QualType ArgTy = ArgExp->getType();
 
     // First see if we can just cast to record type, or point to record type.
@@ -329,7 +338,7 @@ static bool checkAttrArgsAreLockableObjs(Sema &S, Decl *D,
         if(!ArgValue.isStrictlyPositive() || ParamIdxFromOne > NumParams) {
           S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_range)
             << Attr.getName() << Idx + 1 << NumParams;
-          return false;
+          continue;
         }
         ArgTy = FD->getParamDecl(ParamIdxFromZero)->getType();
       }
@@ -339,7 +348,6 @@ static bool checkAttrArgsAreLockableObjs(Sema &S, Decl *D,
 
     Args.push_back(ArgExp);
   }
-  return true;
 }
 
 //===----------------------------------------------------------------------===//
@@ -364,7 +372,7 @@ static void handleGuardedVarAttr(Sema &S, Decl *D, const AttributeList &Attr,
     return;
   }
 
-  if (pointer && !checkIsPointer(S, D, Attr))
+  if (pointer && !threadSafetyCheckIsPointer(S, D, Attr))
     return;
 
   if (pointer)
@@ -380,8 +388,6 @@ static void handleGuardedByAttr(Sema &S, Decl *D, const AttributeList &Attr,
   if (!checkAttributeNumArgs(S, Attr, 1))
     return;
 
-  Expr *Arg = Attr.getArg(0);
-
   // D must be either a member field or global (potentially shared) variable.
   if (!mayBeSharedVariable(D)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
@@ -389,12 +395,16 @@ static void handleGuardedByAttr(Sema &S, Decl *D, const AttributeList &Attr,
     return;
   }
 
-  if (pointer && !checkIsPointer(S, D, Attr))
+  if (pointer && !threadSafetyCheckIsPointer(S, D, Attr))
     return;
 
-  if (!Arg->isTypeDependent()) {
-    checkForLockableRecord(S, D, Attr, Arg->getType());
-  }
+  SmallVector<Expr*, 1> Args;
+  // check that all arguments are lockable objects
+  checkAttrArgsAreLockableObjs(S, D, Attr, Args);
+  unsigned Size = Args.size();
+  if (Size != 1)
+    return;
+  Expr *Arg = Args[0];
 
   if (pointer)
     D->addAttr(::new (S.Context) PtGuardedByAttr(Attr.getRange(),
@@ -442,7 +452,7 @@ static void handleNoThreadSafetyAttr(Sema &S, Decl *D,
 }
 
 static void handleNoAddressSafetyAttr(Sema &S, Decl *D,
-                                     const AttributeList &Attr) {
+                                      const AttributeList &Attr) {
   assert(!Attr.isInvalid());
 
   if (!checkAttributeNumArgs(S, Attr, 0))
@@ -473,25 +483,24 @@ static void handleAcquireOrderAttr(Sema &S, Decl *D, const AttributeList &Attr,
     return;
   }
 
-  // Check that this attribute only applies to lockable types
+  // Check that this attribute only applies to lockable types.
   QualType QT = VD->getType();
   if (!QT->isDependentType()) {
     const RecordType *RT = getRecordType(QT);
     if (!RT || !RT->getDecl()->getAttr<LockableAttr>()) {
-      S.Diag(Attr.getLoc(), diag::warn_attribute_decl_not_lockable)
+      S.Diag(Attr.getLoc(), diag::warn_thread_attribute_decl_not_lockable)
               << Attr.getName();
       return;
     }
   }
 
   SmallVector<Expr*, 1> Args;
-  // check that all arguments are lockable objects
-  if (!checkAttrArgsAreLockableObjs(S, D, Attr, Args))
-    return;
-
+  // Check that all arguments are lockable objects.
+  checkAttrArgsAreLockableObjs(S, D, Attr, Args);
   unsigned Size = Args.size();
-  assert(Size == Attr.getNumArgs());
-  Expr **StartArg = Size == 0 ? 0 : &Args[0];
+  if (Size == 0)
+    return;
+  Expr **StartArg = &Args[0];
 
   if (before)
     D->addAttr(::new (S.Context) AcquiredBeforeAttr(Attr.getRange(), S.Context,
@@ -516,11 +525,8 @@ static void handleLockFunAttr(Sema &S, Decl *D, const AttributeList &Attr,
 
   // check that all arguments are lockable objects
   SmallVector<Expr*, 1> Args;
-  if (!checkAttrArgsAreLockableObjs(S, D, Attr, Args, 0, /*ParamIdxOk=*/true))
-    return;
-
+  checkAttrArgsAreLockableObjs(S, D, Attr, Args, 0, /*ParamIdxOk=*/true);
   unsigned Size = Args.size();
-  assert(Size == Attr.getNumArgs());
   Expr **StartArg = Size == 0 ? 0 : &Args[0];
 
   if (exclusive)
@@ -540,7 +546,6 @@ static void handleTrylockFunAttr(Sema &S, Decl *D, const AttributeList &Attr,
   if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
     return;
 
-
   if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) {
     S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
       << Attr.getName() << ExpectedFunctionOrMethod;
@@ -555,9 +560,7 @@ static void handleTrylockFunAttr(Sema &S, Decl *D, const AttributeList &Attr,
 
   SmallVector<Expr*, 2> Args;
   // check that all arguments are lockable objects
-  if (!checkAttrArgsAreLockableObjs(S, D, Attr, Args, 1))
-    return;
-
+  checkAttrArgsAreLockableObjs(S, D, Attr, Args, 1);
   unsigned Size = Args.size();
   Expr **StartArg = Size == 0 ? 0 : &Args[0];
 
@@ -588,12 +591,11 @@ static void handleLocksRequiredAttr(Sema &S, Decl *D, const AttributeList &Attr,
 
   // check that all arguments are lockable objects
   SmallVector<Expr*, 1> Args;
-  if (!checkAttrArgsAreLockableObjs(S, D, Attr, Args))
-    return;
-
+  checkAttrArgsAreLockableObjs(S, D, Attr, Args);
   unsigned Size = Args.size();
-  assert(Size == Attr.getNumArgs());
-  Expr **StartArg = Size == 0 ? 0 : &Args[0];
+  if (Size == 0)
+    return;
+  Expr **StartArg = &Args[0];
 
   if (exclusive)
     D->addAttr(::new (S.Context) ExclusiveLocksRequiredAttr(Attr.getRange(),
@@ -619,11 +621,8 @@ static void handleUnlockFunAttr(Sema &S, Decl *D,
 
   // check that all arguments are lockable objects
   SmallVector<Expr*, 1> Args;
-  if (!checkAttrArgsAreLockableObjs(S, D, Attr, Args, 0, /*ParamIdxOk=*/true))
-    return;
-
+  checkAttrArgsAreLockableObjs(S, D, Attr, Args, 0, /*ParamIdxOk=*/true);
   unsigned Size = Args.size();
-  assert(Size == Attr.getNumArgs());
   Expr **StartArg = Size == 0 ? 0 : &Args[0];
 
   D->addAttr(::new (S.Context) UnlockFunctionAttr(Attr.getRange(), S.Context,
@@ -668,12 +667,11 @@ static void handleLocksExcludedAttr(Sema &S, Decl *D,
 
   // check that all arguments are lockable objects
   SmallVector<Expr*, 1> Args;
-  if (!checkAttrArgsAreLockableObjs(S, D, Attr, Args))
-    return;
-
+  checkAttrArgsAreLockableObjs(S, D, Attr, Args);
   unsigned Size = Args.size();
-  assert(Size == Attr.getNumArgs());
-  Expr **StartArg = Size == 0 ? 0 : &Args[0];
+  if (Size == 0)
+    return;
+  Expr **StartArg = &Args[0];
 
   D->addAttr(::new (S.Context) LocksExcludedAttr(Attr.getRange(), S.Context,
                                                  StartArg, Size));