r214089 - Thread Safety Analysis: Replace the old and broken SExpr with the new

Delesley Hutchins delesley at google.com
Mon Jul 28 12:08:50 PDT 2014


Thanks!  Is there any way to get MSVC warnings like this in clang, so
I can see them before commit?

  -DeLesley


On Mon, Jul 28, 2014 at 10:58 AM, Reid Kleckner <rnk at google.com> wrote:
> This commit instantiated compareByCase which emitted this warning under
> MSVC:
> d:\src\llvm\tools\clang\include\clang\analysis\analyses\threadsafetytraverse.h(436)
> : warning C4715:
> 'clang::threadSafety::til::Comparator<clang::threadSafety::til::MatchComparator>::compareByCase'
> : not all control paths return a value
>
> I added an unreachable after the switch in r214103 to fix it.
>
>
> On Mon, Jul 28, 2014 at 8:57 AM, DeLesley Hutchins <delesley at google.com>
> wrote:
>>
>> Author: delesley
>> Date: Mon Jul 28 10:57:27 2014
>> New Revision: 214089
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=214089&view=rev
>> Log:
>> Thread Safety Analysis:  Replace the old and broken SExpr with the new
>> til::SExpr.  This is a large patch, with many small changes to pretty
>> printing
>> and expression lowering to make the new SExpr representation equivalent in
>> functionality to the old.
>>
>> Modified:
>>     cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
>>     cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
>>     cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
>>     cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
>>     cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyUtil.h
>>     cfe/trunk/lib/Analysis/ThreadSafety.cpp
>>     cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
>>     cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp
>>     cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
>>     cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
>>
>> Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h (original)
>> +++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h Mon Jul 28
>> 10:57:27 2014
>> @@ -24,7 +24,7 @@
>>  #include "llvm/ADT/StringRef.h"
>>
>>  namespace clang {
>> -namespace thread_safety {
>> +namespace threadSafety {
>>
>>  /// This enum distinguishes between different kinds of operations that
>> may
>>  /// need to be protected by locks. We use this enum in error handling.
>> @@ -190,5 +190,5 @@ void runThreadSafetyAnalysis(AnalysisDec
>>  /// of access.
>>  LockKind getLockKindFromAccessKind(AccessKind AK);
>>
>> -}} // end namespace clang::thread_safety
>> +}} // end namespace clang::threadSafety
>>  #endif
>>
>> Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
>> (original)
>> +++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h Mon Jul
>> 28 10:57:27 2014
>> @@ -219,18 +219,16 @@ public:
>>    /// should be evaluated; multiple calling contexts can be chained
>> together
>>    /// by the lock_returned attribute.
>>    struct CallingContext {
>> +    CallingContext  *Prev;      // The previous context; or 0 if none.
>>      const NamedDecl *AttrDecl;  // The decl to which the attr is
>> attached.
>>      const Expr *SelfArg;        // Implicit object argument -- e.g.
>> 'this'
>>      unsigned NumArgs;           // Number of funArgs
>>      const Expr *const *FunArgs; // Function arguments
>> -    CallingContext *Prev;       // The previous context; or 0 if none.
>>      bool SelfArrow;             // is Self referred to with -> or .?
>>
>> -    CallingContext(const NamedDecl *D = nullptr, const Expr *S = nullptr,
>> -                   unsigned N = 0, const Expr *const *A = nullptr,
>> -                   CallingContext *P = nullptr)
>> -        : AttrDecl(D), SelfArg(S), NumArgs(N), FunArgs(A), Prev(P),
>> -          SelfArrow(false)
>> +    CallingContext(CallingContext *P, const NamedDecl *D = nullptr)
>> +        : Prev(P), AttrDecl(D), SelfArg(nullptr),
>> +          NumArgs(0), FunArgs(nullptr), SelfArrow(false)
>>      {}
>>    };
>>
>> @@ -242,6 +240,13 @@ public:
>>      SelfVar->setKind(til::Variable::VK_SFun);
>>    }
>>
>> +  // Translate a clang expression in an attribute to a til::SExpr.
>> +  // Constructs the context from D, DeclExp, and SelfDecl.
>> +  til::SExpr *translateAttrExpr(const Expr *AttrExp, const NamedDecl *D,
>> +                                const Expr *DeclExp, VarDecl
>> *SelfDecl=nullptr);
>> +
>> +  til::SExpr *translateAttrExpr(const Expr *AttrExp, CallingContext
>> *Ctx);
>> +
>>    // Translate a clang statement or expression to a TIL expression.
>>    // Also performs substitution of variables; Ctx provides the context.
>>    // Dispatches on the type of S.
>> @@ -262,7 +267,8 @@ private:
>>                                     CallingContext *Ctx) ;
>>    til::SExpr *translateCXXThisExpr(const CXXThisExpr *TE, CallingContext
>> *Ctx);
>>    til::SExpr *translateMemberExpr(const MemberExpr *ME, CallingContext
>> *Ctx);
>> -  til::SExpr *translateCallExpr(const CallExpr *CE, CallingContext *Ctx);
>> +  til::SExpr *translateCallExpr(const CallExpr *CE, CallingContext *Ctx,
>> +                                const Expr *SelfE = nullptr);
>>    til::SExpr *translateCXXMemberCallExpr(const CXXMemberCallExpr *ME,
>>                                           CallingContext *Ctx);
>>    til::SExpr *translateCXXOperatorCallExpr(const CXXOperatorCallExpr
>> *OCE,
>> @@ -280,10 +286,8 @@ private:
>>    til::SExpr *translateCastExpr(const CastExpr *CE, CallingContext *Ctx);
>>    til::SExpr *translateArraySubscriptExpr(const ArraySubscriptExpr *E,
>>                                            CallingContext *Ctx);
>> -  til::SExpr *translateConditionalOperator(const ConditionalOperator *C,
>> -                                           CallingContext *Ctx);
>> -  til::SExpr *translateBinaryConditionalOperator(
>> -      const BinaryConditionalOperator *C, CallingContext *Ctx);
>> +  til::SExpr *translateAbstractConditionalOperator(
>> +      const AbstractConditionalOperator *C, CallingContext *Ctx);
>>
>>    til::SExpr *translateDeclStmt(const DeclStmt *S, CallingContext *Ctx);
>>
>> @@ -362,16 +366,19 @@ private:
>>    void mergePhiNodesBackEdge(const CFGBlock *Blk);
>>
>>  private:
>> +  // Set to true when parsing capability expressions, which get
>> translated
>> +  // inaccurately in order to hack around smart pointers etc.
>> +  static const bool CapabilityExprMode = true;
>> +
>>    til::MemRegionRef Arena;
>>    til::Variable *SelfVar;       // Variable to use for 'this'.  May be
>> null.
>> -  til::SCFG *Scfg;
>>
>> +  til::SCFG *Scfg;
>>    StatementMap SMap;                       // Map from Stmt to TIL
>> Variables
>>    LVarIndexMap LVarIdxMap;                 // Indices of clang local
>> vars.
>>    std::vector<til::BasicBlock *> BlockMap; // Map from clang to til BBs.
>>    std::vector<BlockInfo> BBInfo;           // Extra information per BB.
>>                                             // Indexed by clang BlockID.
>> -  std::unique_ptr<SExprBuilder::CallingContext> CallCtx; // Root calling
>> context
>>
>>    LVarDefinitionMap CurrentLVarMap;
>>    std::vector<til::Variable*> CurrentArguments;
>>
>> Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h (original)
>> +++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h Mon Jul 28
>> 10:57:27 2014
>> @@ -100,6 +100,7 @@ enum TIL_CastOpcode : unsigned char {
>>    CAST_truncNum,    // truncate precision of numeric type
>>    CAST_toFloat,     // convert to floating point type
>>    CAST_toInt,       // convert to integer type
>> +  CAST_objToPtr     // convert smart pointer to pointer  (C++ only)
>>  };
>>
>>  const TIL_Opcode       COP_Min  = COP_Future;
>> @@ -405,7 +406,8 @@ public:
>>      return Vs.reduceVariableRef(this);
>>    }
>>
>> -  template <class C> typename C::CType compare(Variable* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Variable* E, C& Cmp) const {
>>      return Cmp.compareVariableRefs(this, E);
>>    }
>>
>> @@ -455,7 +457,7 @@ public:
>>    virtual SExpr *create() { return nullptr; }
>>
>>    // Return the result of this future if it exists, otherwise return
>> null.
>> -  SExpr *maybeGetResult() {
>> +  SExpr *maybeGetResult() const {
>>      return Result;
>>    }
>>
>> @@ -478,7 +480,8 @@ public:
>>      return Vs.traverse(Result, Ctx);
>>    }
>>
>> -  template <class C> typename C::CType compare(Future* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Future* E, C& Cmp) const {
>>      if (!Result || !E->Result)
>>        return Cmp.comparePointers(this, E);
>>      return Cmp.compare(Result, E->Result);
>> @@ -572,8 +575,9 @@ public:
>>      return Vs.reduceUndefined(*this);
>>    }
>>
>> -  template <class C> typename C::CType compare(Undefined* E, C& Cmp) {
>> -    return Cmp.comparePointers(Cstmt, E->Cstmt);
>> +  template <class C>
>> +  typename C::CType compare(const Undefined* E, C& Cmp) const {
>> +    return Cmp.trueResult();
>>    }
>>
>>  private:
>> @@ -593,7 +597,8 @@ public:
>>      return Vs.reduceWildcard(*this);
>>    }
>>
>> -  template <class C> typename C::CType compare(Wildcard* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Wildcard* E, C& Cmp) const {
>>      return Cmp.trueResult();
>>    }
>>  };
>> @@ -626,9 +631,10 @@ public:
>>
>>    template <class V> typename V::R_SExpr traverse(V &Vs, typename
>> V::R_Ctx Ctx);
>>
>> -  template <class C> typename C::CType compare(Literal* E, C& Cmp) {
>> -    // TODO -- use value, not pointer equality
>> -    return Cmp.comparePointers(Cexpr, E->Cexpr);
>> +  template <class C>
>> +  typename C::CType compare(const Literal* E, C& Cmp) const {
>> +    // TODO: defer actual comparison to LiteralT
>> +    return Cmp.trueResult();
>>    }
>>
>>  private:
>> @@ -727,7 +733,8 @@ public:
>>      return Vs.reduceLiteralPtr(*this);
>>    }
>>
>> -  template <class C> typename C::CType compare(LiteralPtr* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const LiteralPtr* E, C& Cmp) const {
>>      return Cmp.comparePointers(Cvdecl, E->Cvdecl);
>>    }
>>
>> @@ -769,7 +776,8 @@ public:
>>      return Vs.reduceFunction(*this, Nvd, E1);
>>    }
>>
>> -  template <class C> typename C::CType compare(Function* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Function* E, C& Cmp) const {
>>      typename C::CType Ct =
>>        Cmp.compare(VarDecl->definition(), E->VarDecl->definition());
>>      if (Cmp.notTrue(Ct))
>> @@ -824,7 +832,8 @@ public:
>>      return Vs.reduceSFunction(*this, Nvd, E1);
>>    }
>>
>> -  template <class C> typename C::CType compare(SFunction* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const SFunction* E, C& Cmp) const {
>>      Cmp.enterScope(variableDecl(), E->variableDecl());
>>      typename C::CType Ct = Cmp.compare(body(), E->body());
>>      Cmp.leaveScope();
>> @@ -859,7 +868,8 @@ public:
>>      return Vs.reduceCode(*this, Nt, Nb);
>>    }
>>
>> -  template <class C> typename C::CType compare(Code* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Code* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(returnType(), E->returnType());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -894,7 +904,8 @@ public:
>>      return Vs.reduceField(*this, Nr, Nb);
>>    }
>>
>> -  template <class C> typename C::CType compare(Field* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Field* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(range(), E->range());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -930,7 +941,8 @@ public:
>>      return Vs.reduceApply(*this, Nf, Na);
>>    }
>>
>> -  template <class C> typename C::CType compare(Apply* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Apply* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(fun(), E->fun());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -958,7 +970,7 @@ public:
>>    SExpr *arg() { return Arg.get() ? Arg.get() : Sfun.get(); }
>>    const SExpr *arg() const { return Arg.get() ? Arg.get() : Sfun.get(); }
>>
>> -  bool isDelegation() const { return Arg == nullptr; }
>> +  bool isDelegation() const { return Arg != nullptr; }
>>
>>    template <class V>
>>    typename V::R_SExpr traverse(V &Vs, typename V::R_Ctx Ctx) {
>> @@ -968,7 +980,8 @@ public:
>>      return Vs.reduceSApply(*this, Nf, Na);
>>    }
>>
>> -  template <class C> typename C::CType compare(SApply* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const SApply* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(sfun(), E->sfun());
>>      if (Cmp.notTrue(Ct) || (!arg() && !E->arg()))
>>        return Ct;
>> @@ -989,7 +1002,7 @@ public:
>>    Project(SExpr *R, StringRef SName)
>>        : SExpr(COP_Project), Rec(R), SlotName(SName), Cvdecl(nullptr)
>>    { }
>> -  Project(SExpr *R, clang::ValueDecl *Cvd)
>> +  Project(SExpr *R, const clang::ValueDecl *Cvd)
>>        : SExpr(COP_Project), Rec(R), SlotName(Cvd->getName()), Cvdecl(Cvd)
>>    { }
>>    Project(const Project &P, SExpr *R)
>> @@ -999,7 +1012,13 @@ public:
>>    SExpr *record() { return Rec.get(); }
>>    const SExpr *record() const { return Rec.get(); }
>>
>> -  const clang::ValueDecl *clangValueDecl() const { return Cvdecl; }
>> +  const clang::ValueDecl *clangDecl() const { return Cvdecl; }
>> +
>> +  bool isArrow() const { return (Flags & 0x01) != 0; }
>> +  void setArrow(bool b) {
>> +    if (b) Flags |= 0x01;
>> +    else Flags &= 0xFFFE;
>> +  }
>>
>>    StringRef slotName() const {
>>      if (Cvdecl)
>> @@ -1014,7 +1033,8 @@ public:
>>      return Vs.reduceProject(*this, Nr);
>>    }
>>
>> -  template <class C> typename C::CType compare(Project* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Project* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(record(), E->record());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -1024,7 +1044,7 @@ public:
>>  private:
>>    SExprRef Rec;
>>    StringRef SlotName;
>> -  clang::ValueDecl *Cvdecl;
>> +  const clang::ValueDecl *Cvdecl;
>>  };
>>
>>
>> @@ -1048,7 +1068,8 @@ public:
>>      return Vs.reduceCall(*this, Nt);
>>    }
>>
>> -  template <class C> typename C::CType compare(Call* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Call* E, C& Cmp) const {
>>      return Cmp.compare(target(), E->target());
>>    }
>>
>> @@ -1082,7 +1103,8 @@ public:
>>      return Vs.reduceAlloc(*this, Nd);
>>    }
>>
>> -  template <class C> typename C::CType compare(Alloc* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Alloc* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compareIntegers(kind(), E->kind());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -1111,7 +1133,8 @@ public:
>>      return Vs.reduceLoad(*this, Np);
>>    }
>>
>> -  template <class C> typename C::CType compare(Load* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Load* E, C& Cmp) const {
>>      return Cmp.compare(pointer(), E->pointer());
>>    }
>>
>> @@ -1142,7 +1165,8 @@ public:
>>      return Vs.reduceStore(*this, Np, Nv);
>>    }
>>
>> -  template <class C> typename C::CType compare(Store* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Store* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(destination(), E->destination());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -1178,7 +1202,8 @@ public:
>>      return Vs.reduceArrayIndex(*this, Na, Ni);
>>    }
>>
>> -  template <class C> typename C::CType compare(ArrayIndex* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const ArrayIndex* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(array(), E->array());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -1215,7 +1240,8 @@ public:
>>      return Vs.reduceArrayAdd(*this, Na, Ni);
>>    }
>>
>> -  template <class C> typename C::CType compare(ArrayAdd* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const ArrayAdd* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(array(), E->array());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -1251,7 +1277,8 @@ public:
>>      return Vs.reduceUnaryOp(*this, Ne);
>>    }
>>
>> -  template <class C> typename C::CType compare(UnaryOp* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const UnaryOp* E, C& Cmp) const {
>>      typename C::CType Ct =
>>        Cmp.compareIntegers(unaryOpcode(), E->unaryOpcode());
>>      if (Cmp.notTrue(Ct))
>> @@ -1295,7 +1322,8 @@ public:
>>      return Vs.reduceBinaryOp(*this, Ne0, Ne1);
>>    }
>>
>> -  template <class C> typename C::CType compare(BinaryOp* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const BinaryOp* E, C& Cmp) const {
>>      typename C::CType Ct =
>>        Cmp.compareIntegers(binaryOpcode(), E->binaryOpcode());
>>      if (Cmp.notTrue(Ct))
>> @@ -1333,7 +1361,8 @@ public:
>>      return Vs.reduceCast(*this, Ne);
>>    }
>>
>> -  template <class C> typename C::CType compare(Cast* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Cast* E, C& Cmp) const {
>>      typename C::CType Ct =
>>        Cmp.compareIntegers(castOpcode(), E->castOpcode());
>>      if (Cmp.notTrue(Ct))
>> @@ -1386,7 +1415,8 @@ public:
>>      return Vs.reducePhi(*this, Nvs);
>>    }
>>
>> -  template <class C> typename C::CType compare(Phi *E, C &Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Phi *E, C &Cmp) const {
>>      // TODO: implement CFG comparisons
>>      return Cmp.comparePointers(this, E);
>>    }
>> @@ -1503,7 +1533,8 @@ public:
>>      return Vs.reduceBasicBlock(*this, Nas, Nis, Nt);
>>    }
>>
>> -  template <class C> typename C::CType compare(BasicBlock *E, C &Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const BasicBlock *E, C &Cmp) const {
>>      // TODO: implement CFG comparisons
>>      return Cmp.comparePointers(this, E);
>>    }
>> @@ -1590,7 +1621,8 @@ public:
>>      return Vs.reduceSCFG(*this, Bbs);
>>    }
>>
>> -  template <class C> typename C::CType compare(SCFG *E, C &Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const SCFG *E, C &Cmp) const {
>>      // TODO -- implement CFG comparisons
>>      return Cmp.comparePointers(this, E);
>>    }
>> @@ -1623,7 +1655,8 @@ public:
>>      return Vs.reduceGoto(*this, Ntb);
>>    }
>>
>> -  template <class C> typename C::CType compare(Goto *E, C &Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Goto *E, C &Cmp) const {
>>      // TODO -- implement CFG comparisons
>>      return Cmp.comparePointers(this, E);
>>    }
>> @@ -1668,7 +1701,8 @@ public:
>>      return Vs.reduceBranch(*this, Nc, Ntb, Nte);
>>    }
>>
>> -  template <class C> typename C::CType compare(Branch *E, C &Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Branch *E, C &Cmp) const {
>>      // TODO -- implement CFG comparisons
>>      return Cmp.comparePointers(this, E);
>>    }
>> @@ -1698,7 +1732,8 @@ public:
>>      return Vs.reduceIdentifier(*this);
>>    }
>>
>> -  template <class C> typename C::CType compare(Identifier* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Identifier* E, C& Cmp) const {
>>      return Cmp.compareStrings(name(), E->name());
>>    }
>>
>> @@ -1737,7 +1772,8 @@ public:
>>      return Vs.reduceIfThenElse(*this, Nc, Nt, Ne);
>>    }
>>
>> -  template <class C> typename C::CType compare(IfThenElse* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const IfThenElse* E, C& Cmp) const {
>>      typename C::CType Ct = Cmp.compare(condition(), E->condition());
>>      if (Cmp.notTrue(Ct))
>>        return Ct;
>> @@ -1784,7 +1820,8 @@ public:
>>      return Vs.reduceLet(*this, Nvd, E1);
>>    }
>>
>> -  template <class C> typename C::CType compare(Let* E, C& Cmp) {
>> +  template <class C>
>> +  typename C::CType compare(const Let* E, C& Cmp) const {
>>      typename C::CType Ct =
>>        Cmp.compare(VarDecl->definition(), E->VarDecl->definition());
>>      if (Cmp.notTrue(Ct))
>> @@ -1802,7 +1839,8 @@ private:
>>
>>
>>
>> -SExpr *getCanonicalVal(SExpr *E);
>> +const SExpr *getCanonicalVal(const SExpr *E);
>> +SExpr* simplifyToCanonicalVal(SExpr *E);
>>  void simplifyIncompleteArg(Variable *V, til::Phi *Ph);
>>
>>
>>
>> Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
>> (original)
>> +++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h Mon
>> Jul 28 10:57:27 2014
>> @@ -19,6 +19,8 @@
>>
>>  #include "ThreadSafetyTIL.h"
>>
>> +#include <ostream>
>> +
>>  namespace clang {
>>  namespace threadSafety {
>>  namespace til {
>> @@ -423,7 +425,7 @@ protected:
>>    Self *self() { return reinterpret_cast<Self *>(this); }
>>
>>  public:
>> -  bool compareByCase(SExpr *E1, SExpr* E2) {
>> +  bool compareByCase(const SExpr *E1, const SExpr* E2) {
>>      switch (E1->opcode()) {
>>  #define TIL_OPCODE_DEF(X)
>> \
>>      case COP_##X:
>> \
>> @@ -449,38 +451,86 @@ public:
>>    bool compareStrings (StringRef s, StringRef r)     { return s == r; }
>>    bool comparePointers(const void* P, const void* Q) { return P == Q; }
>>
>> -  bool compare(SExpr *E1, SExpr* E2) {
>> +  bool compare(const SExpr *E1, const SExpr* E2) {
>>      if (E1->opcode() != E2->opcode())
>>        return false;
>>      return compareByCase(E1, E2);
>>    }
>>
>>    // TODO -- handle alpha-renaming of variables
>> -  void enterScope(Variable* V1, Variable* V2) { }
>> +  void enterScope(const Variable* V1, const Variable* V2) { }
>>    void leaveScope() { }
>>
>> -  bool compareVariableRefs(Variable* V1, Variable* V2) {
>> +  bool compareVariableRefs(const Variable* V1, const Variable* V2) {
>>      return V1 == V2;
>>    }
>>
>> -  static bool compareExprs(SExpr *E1, SExpr* E2) {
>> +  static bool compareExprs(const SExpr *E1, const SExpr* E2) {
>>      EqualsComparator Eq;
>>      return Eq.compare(E1, E2);
>>    }
>>  };
>>
>>
>> +
>> +class MatchComparator : public Comparator<MatchComparator> {
>> +public:
>> +  // Result type for the comparison, e.g. bool for simple equality,
>> +  // or int for lexigraphic comparison (-1, 0, 1).  Must have one value
>> which
>> +  // denotes "true".
>> +  typedef bool CType;
>> +
>> +  CType trueResult() { return true; }
>> +  bool notTrue(CType ct) { return !ct; }
>> +
>> +  bool compareIntegers(unsigned i, unsigned j)       { return i == j; }
>> +  bool compareStrings (StringRef s, StringRef r)     { return s == r; }
>> +  bool comparePointers(const void* P, const void* Q) { return P == Q; }
>> +
>> +  bool compare(const SExpr *E1, const SExpr* E2) {
>> +    // Wildcards match anything.
>> +    if (E1->opcode() == COP_Wildcard || E2->opcode() == COP_Wildcard)
>> +      return true;
>> +    // otherwise normal equality.
>> +    if (E1->opcode() != E2->opcode())
>> +      return false;
>> +    return compareByCase(E1, E2);
>> +  }
>> +
>> +  // TODO -- handle alpha-renaming of variables
>> +  void enterScope(const Variable* V1, const Variable* V2) { }
>> +  void leaveScope() { }
>> +
>> +  bool compareVariableRefs(const Variable* V1, const Variable* V2) {
>> +    return V1 == V2;
>> +  }
>> +
>> +  static bool compareExprs(const SExpr *E1, const SExpr* E2) {
>> +    MatchComparator Matcher;
>> +    return Matcher.compare(E1, E2);
>> +  }
>> +};
>> +
>> +
>> +
>> +inline std::ostream& operator<<(std::ostream& SS, llvm::StringRef R) {
>> +  return SS.write(R.data(), R.size());
>> +}
>> +
>>  // Pretty printer for TIL expressions
>>  template <typename Self, typename StreamType>
>>  class PrettyPrinter {
>>  private:
>>    bool Verbose;  // Print out additional information
>>    bool Cleanup;  // Omit redundant decls.
>> +  bool CStyle;   // Print exprs in C-like syntax.
>>
>>  public:
>> -  PrettyPrinter(bool V = false, bool C = true) : Verbose(V), Cleanup(C) {
>> }
>> +  PrettyPrinter(bool V = false, bool C = true, bool CS = true)
>> +     : Verbose(V), Cleanup(C), CStyle(CS)
>> +  {}
>>
>> -  static void print(SExpr *E, StreamType &SS) {
>> +  static void print(const SExpr *E, StreamType &SS) {
>>      Self printer;
>>      printer.printSExpr(E, SS, Prec_MAX);
>>    }
>> @@ -502,7 +552,7 @@ protected:
>>    static const unsigned Prec_MAX = 6;
>>
>>    // Return the precedence of a given node, for use in pretty printing.
>> -  unsigned precedence(SExpr *E) {
>> +  unsigned precedence(const SExpr *E) {
>>      switch (E->opcode()) {
>>        case COP_Future:     return Prec_Atom;
>>        case COP_Undefined:  return Prec_Atom;
>> @@ -529,7 +579,7 @@ protected:
>>
>>        case COP_UnaryOp:    return Prec_Unary;
>>        case COP_BinaryOp:   return Prec_Binary;
>> -      case COP_Cast:       return Prec_Unary;
>> +      case COP_Cast:       return Prec_Atom;
>>
>>        case COP_SCFG:       return Prec_Decl;
>>        case COP_BasicBlock: return Prec_MAX;
>> @@ -544,7 +594,7 @@ protected:
>>      return Prec_MAX;
>>    }
>>
>> -  void printBlockLabel(StreamType & SS, BasicBlock *BB, unsigned index) {
>> +  void printBlockLabel(StreamType & SS, const BasicBlock *BB, unsigned
>> index) {
>>      if (!BB) {
>>        SS << "BB_null";
>>        return;
>> @@ -555,7 +605,7 @@ protected:
>>      SS << index;
>>    }
>>
>> -  void printSExpr(SExpr *E, StreamType &SS, unsigned P) {
>> +  void printSExpr(const SExpr *E, StreamType &SS, unsigned P) {
>>      if (!E) {
>>        self()->printNull(SS);
>>        return;
>> @@ -582,28 +632,28 @@ protected:
>>      SS << "#null";
>>    }
>>
>> -  void printFuture(Future *E, StreamType &SS) {
>> +  void printFuture(const Future *E, StreamType &SS) {
>>      self()->printSExpr(E->maybeGetResult(), SS, Prec_Atom);
>>    }
>>
>> -  void printUndefined(Undefined *E, StreamType &SS) {
>> +  void printUndefined(const Undefined *E, StreamType &SS) {
>>      SS << "#undefined";
>>    }
>>
>> -  void printWildcard(Wildcard *E, StreamType &SS) {
>> -    SS << "_";
>> +  void printWildcard(const Wildcard *E, StreamType &SS) {
>> +    SS << "*";
>>    }
>>
>>    template<class T>
>> -  void printLiteralT(LiteralT<T> *E, StreamType &SS) {
>> +  void printLiteralT(const LiteralT<T> *E, StreamType &SS) {
>>      SS << E->value();
>>    }
>>
>> -  void printLiteralT(LiteralT<uint8_t> *E, StreamType &SS) {
>> +  void printLiteralT(const LiteralT<uint8_t> *E, StreamType &SS) {
>>      SS << "'" << E->value() << "'";
>>    }
>>
>> -  void printLiteral(Literal *E, StreamType &SS) {
>> +  void printLiteral(const Literal *E, StreamType &SS) {
>>      if (E->clangExpr()) {
>>        SS << getSourceLiteralString(E->clangExpr());
>>        return;
>> @@ -685,13 +735,13 @@ protected:
>>      SS << "#lit";
>>    }
>>
>> -  void printLiteralPtr(LiteralPtr *E, StreamType &SS) {
>> +  void printLiteralPtr(const LiteralPtr *E, StreamType &SS) {
>>      SS << E->clangDecl()->getNameAsString();
>>    }
>>
>> -  void printVariable(Variable *V, StreamType &SS, bool IsVarDecl = false)
>> {
>> +  void printVariable(const Variable *V, StreamType &SS, bool IsVarDecl =
>> false) {
>>      if (!IsVarDecl && Cleanup) {
>> -      SExpr* E = getCanonicalVal(V);
>> +      const SExpr* E = getCanonicalVal(V);
>>        if (E != V) {
>>          printSExpr(E, SS, Prec_Atom);
>>          return;
>> @@ -699,11 +749,13 @@ protected:
>>      }
>>      if (V->kind() == Variable::VK_LetBB)
>>        SS << V->name() << V->getBlockID() << "_" << V->getID();
>> +    else if (CStyle && V->kind() == Variable::VK_SFun)
>> +      SS << "this";
>>      else
>>        SS << V->name() << V->getID();
>>    }
>>
>> -  void printFunction(Function *E, StreamType &SS, unsigned sugared = 0) {
>> +  void printFunction(const Function *E, StreamType &SS, unsigned sugared
>> = 0) {
>>      switch (sugared) {
>>        default:
>>          SS << "\\(";   // Lambda
>> @@ -719,7 +771,7 @@ protected:
>>      SS << ": ";
>>      self()->printSExpr(E->variableDecl()->definition(), SS, Prec_MAX);
>>
>> -    SExpr *B = E->body();
>> +    const SExpr *B = E->body();
>>      if (B && B->opcode() == COP_Function)
>>        self()->printFunction(cast<Function>(B), SS, 2);
>>      else {
>> @@ -728,29 +780,29 @@ protected:
>>      }
>>    }
>>
>> -  void printSFunction(SFunction *E, StreamType &SS) {
>> +  void printSFunction(const SFunction *E, StreamType &SS) {
>>      SS << "@";
>>      self()->printVariable(E->variableDecl(), SS, true);
>>      SS << " ";
>>      self()->printSExpr(E->body(), SS, Prec_Decl);
>>    }
>>
>> -  void printCode(Code *E, StreamType &SS) {
>> +  void printCode(const Code *E, StreamType &SS) {
>>      SS << ": ";
>>      self()->printSExpr(E->returnType(), SS, Prec_Decl-1);
>>      SS << " -> ";
>>      self()->printSExpr(E->body(), SS, Prec_Decl);
>>    }
>>
>> -  void printField(Field *E, StreamType &SS) {
>> +  void printField(const Field *E, StreamType &SS) {
>>      SS << ": ";
>>      self()->printSExpr(E->range(), SS, Prec_Decl-1);
>>      SS << " = ";
>>      self()->printSExpr(E->body(), SS, Prec_Decl);
>>    }
>>
>> -  void printApply(Apply *E, StreamType &SS, bool sugared = false) {
>> -    SExpr *F = E->fun();
>> +  void printApply(const Apply *E, StreamType &SS, bool sugared = false) {
>> +    const SExpr *F = E->fun();
>>      if (F->opcode() == COP_Apply) {
>>        printApply(cast<Apply>(F), SS, true);
>>        SS << ", ";
>> @@ -763,7 +815,7 @@ protected:
>>        SS << ")$";
>>    }
>>
>> -  void printSApply(SApply *E, StreamType &SS) {
>> +  void printSApply(const SApply *E, StreamType &SS) {
>>      self()->printSExpr(E->sfun(), SS, Prec_Postfix);
>>      if (E->isDelegation()) {
>>        SS << "@(";
>> @@ -772,14 +824,36 @@ protected:
>>      }
>>    }
>>
>> -  void printProject(Project *E, StreamType &SS) {
>> +  void printProject(const Project *E, StreamType &SS) {
>> +    if (CStyle) {
>> +      // Omit the  this->
>> +      if (const SApply *SAP = dyn_cast<SApply>(E->record())) {
>> +        if (const Variable *V = dyn_cast<Variable>(SAP->sfun())) {
>> +          if (!SAP->isDelegation() && V->kind() == Variable::VK_SFun) {
>> +            SS << E->slotName();
>> +            return;
>> +          }
>> +        }
>> +      }
>> +      if (isa<Wildcard>(E->record())) {
>> +        // handle existentials
>> +        SS << "&";
>> +        SS << E->clangDecl()->getQualifiedNameAsString();
>> +        return;
>> +      }
>> +    }
>>      self()->printSExpr(E->record(), SS, Prec_Postfix);
>> -    SS << ".";
>> +    if (CStyle && E->isArrow()) {
>> +      SS << "->";
>> +    }
>> +    else {
>> +      SS << ".";
>> +    }
>>      SS << E->slotName();
>>    }
>>
>> -  void printCall(Call *E, StreamType &SS) {
>> -    SExpr *T = E->target();
>> +  void printCall(const Call *E, StreamType &SS) {
>> +    const SExpr *T = E->target();
>>      if (T->opcode() == COP_Apply) {
>>        self()->printApply(cast<Apply>(T), SS, true);
>>        SS << ")";
>> @@ -790,52 +864,60 @@ protected:
>>      }
>>    }
>>
>> -  void printAlloc(Alloc *E, StreamType &SS) {
>> +  void printAlloc(const Alloc *E, StreamType &SS) {
>>      SS << "new ";
>>      self()->printSExpr(E->dataType(), SS, Prec_Other-1);
>>    }
>>
>> -  void printLoad(Load *E, StreamType &SS) {
>> +  void printLoad(const Load *E, StreamType &SS) {
>>      self()->printSExpr(E->pointer(), SS, Prec_Postfix);
>> -    SS << "^";
>> +    if (!CStyle)
>> +      SS << "^";
>>    }
>>
>> -  void printStore(Store *E, StreamType &SS) {
>> +  void printStore(const Store *E, StreamType &SS) {
>>      self()->printSExpr(E->destination(), SS, Prec_Other-1);
>>      SS << " := ";
>>      self()->printSExpr(E->source(), SS, Prec_Other-1);
>>    }
>>
>> -  void printArrayIndex(ArrayIndex *E, StreamType &SS) {
>> +  void printArrayIndex(const ArrayIndex *E, StreamType &SS) {
>>      self()->printSExpr(E->array(), SS, Prec_Postfix);
>>      SS << "[";
>>      self()->printSExpr(E->index(), SS, Prec_MAX);
>>      SS << "]";
>>    }
>>
>> -  void printArrayAdd(ArrayAdd *E, StreamType &SS) {
>> +  void printArrayAdd(const ArrayAdd *E, StreamType &SS) {
>>      self()->printSExpr(E->array(), SS, Prec_Postfix);
>>      SS << " + ";
>>      self()->printSExpr(E->index(), SS, Prec_Atom);
>>    }
>>
>> -  void printUnaryOp(UnaryOp *E, StreamType &SS) {
>> +  void printUnaryOp(const UnaryOp *E, StreamType &SS) {
>>      SS << getUnaryOpcodeString(E->unaryOpcode());
>>      self()->printSExpr(E->expr(), SS, Prec_Unary);
>>    }
>>
>> -  void printBinaryOp(BinaryOp *E, StreamType &SS) {
>> +  void printBinaryOp(const BinaryOp *E, StreamType &SS) {
>>      self()->printSExpr(E->expr0(), SS, Prec_Binary-1);
>>      SS << " " << getBinaryOpcodeString(E->binaryOpcode()) << " ";
>>      self()->printSExpr(E->expr1(), SS, Prec_Binary-1);
>>    }
>>
>> -  void printCast(Cast *E, StreamType &SS) {
>> -    SS << "%";
>> +  void printCast(const Cast *E, StreamType &SS) {
>> +    if (!CStyle) {
>> +      SS << "cast[";
>> +      SS << E->castOpcode();
>> +      SS << "](";
>> +      self()->printSExpr(E->expr(), SS, Prec_Unary);
>> +      SS << ")";
>> +      return;
>> +    }
>>      self()->printSExpr(E->expr(), SS, Prec_Unary);
>>    }
>>
>> -  void printSCFG(SCFG *E, StreamType &SS) {
>> +  void printSCFG(const SCFG *E, StreamType &SS) {
>>      SS << "CFG {\n";
>>      for (auto BBI : *E) {
>>        printBasicBlock(BBI, SS);
>> @@ -844,7 +926,7 @@ protected:
>>      newline(SS);
>>    }
>>
>> -  void printBasicBlock(BasicBlock *E, StreamType &SS) {
>> +  void printBasicBlock(const BasicBlock *E, StreamType &SS) {
>>      SS << "BB_" << E->blockID() << ":";
>>      if (E->parent())
>>        SS << " BB_" << E->parent()->blockID();
>> @@ -867,7 +949,7 @@ protected:
>>        SS << ";";
>>        newline(SS);
>>      }
>> -    SExpr *T = E->terminator();
>> +    const SExpr *T = E->terminator();
>>      if (T) {
>>        self()->printSExpr(T, SS, Prec_MAX);
>>        SS << ";";
>> @@ -876,7 +958,7 @@ protected:
>>      newline(SS);
>>    }
>>
>> -  void printPhi(Phi *E, StreamType &SS) {
>> +  void printPhi(const Phi *E, StreamType &SS) {
>>      SS << "phi(";
>>      if (E->status() == Phi::PH_SingleVal)
>>        self()->printSExpr(E->values()[0], SS, Prec_MAX);
>> @@ -891,12 +973,12 @@ protected:
>>      SS << ")";
>>    }
>>
>> -  void printGoto(Goto *E, StreamType &SS) {
>> +  void printGoto(const Goto *E, StreamType &SS) {
>>      SS << "goto ";
>>      printBlockLabel(SS, E->targetBlock(), E->index());
>>    }
>>
>> -  void printBranch(Branch *E, StreamType &SS) {
>> +  void printBranch(const Branch *E, StreamType &SS) {
>>      SS << "branch (";
>>      self()->printSExpr(E->condition(), SS, Prec_MAX);
>>      SS << ") ";
>> @@ -905,11 +987,19 @@ protected:
>>      printBlockLabel(SS, E->elseBlock(), E->elseIndex());
>>    }
>>
>> -  void printIdentifier(Identifier *E, StreamType &SS) {
>> +  void printIdentifier(const Identifier *E, StreamType &SS) {
>>      SS << E->name();
>>    }
>>
>> -  void printIfThenElse(IfThenElse *E, StreamType &SS) {
>> +  void printIfThenElse(const IfThenElse *E, StreamType &SS) {
>> +    if (CStyle) {
>> +      printSExpr(E->condition(), SS, Prec_Unary);
>> +      SS << " ? ";
>> +      printSExpr(E->thenExpr(), SS, Prec_Unary);
>> +      SS << " : ";
>> +      printSExpr(E->elseExpr(), SS, Prec_Unary);
>> +      return;
>> +    }
>>      SS << "if (";
>>      printSExpr(E->condition(), SS, Prec_MAX);
>>      SS << ") then ";
>> @@ -918,7 +1008,7 @@ protected:
>>      printSExpr(E->elseExpr(), SS, Prec_Other);
>>    }
>>
>> -  void printLet(Let *E, StreamType &SS) {
>> +  void printLet(const Let *E, StreamType &SS) {
>>      SS << "let ";
>>      printVariable(E->variableDecl(), SS, true);
>>      SS << " = ";
>> @@ -929,6 +1019,10 @@ protected:
>>  };
>>
>>
>> +class StdPrinter : public PrettyPrinter<StdPrinter, std::ostream> { };
>> +
>> +
>> +
>>  } // end namespace til
>>  } // end namespace threadSafety
>>  } // end namespace clang
>>
>> Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyUtil.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyUtil.h?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyUtil.h
>> (original)
>> +++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyUtil.h Mon Jul
>> 28 10:57:27 2014
>> @@ -144,7 +144,9 @@ public:
>>    }
>>
>>    iterator begin() { return Data; }
>> +  const_iterator begin() const { return Data; }
>>    iterator end() { return Data + Size; }
>> +  const_iterator end() const { return Data + Size; }
>>
>>    const_iterator cbegin() const { return Data; }
>>    const_iterator cend() const { return Data + Size; }
>>
>> Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
>> +++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Mon Jul 28 10:57:27 2014
>> @@ -40,682 +40,107 @@
>>  #include "llvm/ADT/StringRef.h"
>>  #include "llvm/Support/raw_ostream.h"
>>  #include <algorithm>
>> +#include <ostream>
>> +#include <sstream>
>>  #include <utility>
>>  #include <vector>
>>
>> -using namespace clang;
>> -using namespace thread_safety;
>> +
>> +namespace clang {
>> +namespace threadSafety {
>>
>>  // Key method definition
>>  ThreadSafetyHandler::~ThreadSafetyHandler() {}
>>
>> -namespace {
>> -
>> -/// SExpr implements a simple expression language that is used to store,
>> -/// compare, and pretty-print C++ expressions.  Unlike a clang Expr, a
>> SExpr
>> -/// does not capture surface syntax, and it does not distinguish between
>> -/// C++ concepts, like pointers and references, that have no real
>> semantic
>> -/// differences.  This simplicity allows SExprs to be meaningfully
>> compared,
>> -/// e.g.
>> -///        (x)          =  x
>> -///        (*this).foo  =  this->foo
>> -///        *&a          =  a
>> -///
>> -/// Thread-safety analysis works by comparing lock expressions.  Within
>> the
>> -/// body of a function, an expression such as "x->foo->bar.mu" will
>> resolve to
>> -/// a particular mutex object at run-time.  Subsequent occurrences of the
>> same
>> -/// expression (where "same" means syntactic equality) will refer to the
>> same
>> -/// run-time object if three conditions hold:
>> -/// (1) Local variables in the expression, such as "x" have not changed.
>> -/// (2) Values on the heap that affect the expression have not changed.
>> -/// (3) The expression involves only pure function calls.
>> -///
>> -/// The current implementation assumes, but does not verify, that
>> multiple uses
>> -/// of the same lock expression satisfies these criteria.
>> -class SExpr {
>> -private:
>> -  enum ExprOp {
>> -    EOP_Nop,       ///< No-op
>> -    EOP_Wildcard,  ///< Matches anything.
>> -    EOP_Universal, ///< Universal lock.
>> -    EOP_This,      ///< This keyword.
>> -    EOP_NVar,      ///< Named variable.
>> -    EOP_LVar,      ///< Local variable.
>> -    EOP_Dot,       ///< Field access
>> -    EOP_Call,      ///< Function call
>> -    EOP_MCall,     ///< Method call
>> -    EOP_Index,     ///< Array index
>> -    EOP_Unary,     ///< Unary operation
>> -    EOP_Binary,    ///< Binary operation
>> -    EOP_Unknown    ///< Catchall for everything else
>> -  };
>> -
>> -
>> -  class SExprNode {
>> -   private:
>> -    unsigned char  Op;     ///< Opcode of the root node
>> -    unsigned char  Flags;  ///< Additional opcode-specific data
>> -    unsigned short Sz;     ///< Number of child nodes
>> -    const void*    Data;   ///< Additional opcode-specific data
>> -
>> -   public:
>> -    SExprNode(ExprOp O, unsigned F, const void* D)
>> -      : Op(static_cast<unsigned char>(O)),
>> -        Flags(static_cast<unsigned char>(F)), Sz(1), Data(D)
>> -    { }
>> -
>> -    unsigned size() const        { return Sz; }
>> -    void     setSize(unsigned S) { Sz = S;    }
>> -
>> -    ExprOp   kind() const { return static_cast<ExprOp>(Op); }
>> -
>> -    const NamedDecl* getNamedDecl() const {
>> -      assert(Op == EOP_NVar || Op == EOP_LVar || Op == EOP_Dot);
>> -      return reinterpret_cast<const NamedDecl*>(Data);
>> -    }
>> -
>> -    const NamedDecl* getFunctionDecl() const {
>> -      assert(Op == EOP_Call || Op == EOP_MCall);
>> -      return reinterpret_cast<const NamedDecl*>(Data);
>> -    }
>> -
>> -    bool isArrow() const { return Op == EOP_Dot && Flags == 1; }
>> -    void setArrow(bool A) { Flags = A ? 1 : 0; }
>> -
>> -    unsigned arity() const {
>> -      switch (Op) {
>> -        case EOP_Nop:       return 0;
>> -        case EOP_Wildcard:  return 0;
>> -        case EOP_Universal: return 0;
>> -        case EOP_NVar:      return 0;
>> -        case EOP_LVar:      return 0;
>> -        case EOP_This:      return 0;
>> -        case EOP_Dot:       return 1;
>> -        case EOP_Call:      return Flags+1;  // First arg is function.
>> -        case EOP_MCall:     return Flags+1;  // First arg is implicit
>> obj.
>> -        case EOP_Index:     return 2;
>> -        case EOP_Unary:     return 1;
>> -        case EOP_Binary:    return 2;
>> -        case EOP_Unknown:   return Flags;
>> -      }
>> -      return 0;
>> -    }
>> -
>> -    bool operator==(const SExprNode& Other) const {
>> -      // Ignore flags and size -- they don't matter.
>> -      return (Op == Other.Op &&
>> -              Data == Other.Data);
>> -    }
>> -
>> -    bool operator!=(const SExprNode& Other) const {
>> -      return !(*this == Other);
>> -    }
>> -
>> -    bool matches(const SExprNode& Other) const {
>> -      return (*this == Other) ||
>> -             (Op == EOP_Wildcard) ||
>> -             (Other.Op == EOP_Wildcard);
>> -    }
>> -  };
>> -
>> -
>> -  /// \brief Encapsulates the lexical context of a function call.  The
>> lexical
>> -  /// context includes the arguments to the call, including the implicit
>> object
>> -  /// argument.  When an attribute containing a mutex expression is
>> attached to
>> -  /// a method, the expression may refer to formal parameters of the
>> method.
>> -  /// Actual arguments must be substituted for formal parameters to
>> derive
>> -  /// the appropriate mutex expression in the lexical context where the
>> function
>> -  /// is called.  PrevCtx holds the context in which the arguments
>> themselves
>> -  /// should be evaluated; multiple calling contexts can be chained
>> together
>> -  /// by the lock_returned attribute.
>> -  struct CallingContext {
>> -    const NamedDecl*   AttrDecl;   // The decl to which the attribute is
>> attached.
>> -    const Expr*        SelfArg;    // Implicit object argument -- e.g.
>> 'this'
>> -    bool               SelfArrow;  // is Self referred to with -> or .?
>> -    unsigned           NumArgs;    // Number of funArgs
>> -    const Expr* const* FunArgs;    // Function arguments
>> -    CallingContext*    PrevCtx;    // The previous context; or 0 if none.
>> -
>> -    CallingContext(const NamedDecl *D)
>> -        : AttrDecl(D), SelfArg(nullptr), SelfArrow(false), NumArgs(0),
>> -          FunArgs(nullptr), PrevCtx(nullptr) {}
>> -  };
>> -
>> -  typedef SmallVector<SExprNode, 4> NodeVector;
>> -
>> -private:
>> -  // A SExpr is a list of SExprNodes in prefix order.  The Size field
>> allows
>> -  // the list to be traversed as a tree.
>> -  NodeVector NodeVec;
>> -
>> -private:
>> -  unsigned make(ExprOp O, unsigned F = 0, const void *D = nullptr) {
>> -    NodeVec.push_back(SExprNode(O, F, D));
>> -    return NodeVec.size() - 1;
>> -  }
>> -
>> -  unsigned makeNop() {
>> -    return make(EOP_Nop);
>> -  }
>> -
>> -  unsigned makeWildcard() {
>> -    return make(EOP_Wildcard);
>> -  }
>> -
>> -  unsigned makeUniversal() {
>> -    return make(EOP_Universal);
>> -  }
>> +class TILPrinter :
>> +  public til::PrettyPrinter<TILPrinter, llvm::raw_ostream> {};
>>
>> -  unsigned makeNamedVar(const NamedDecl *D) {
>> -    return make(EOP_NVar, 0, D);
>> -  }
>> -
>> -  unsigned makeLocalVar(const NamedDecl *D) {
>> -    return make(EOP_LVar, 0, D);
>> -  }
>> -
>> -  unsigned makeThis() {
>> -    return make(EOP_This);
>> -  }
>>
>> -  unsigned makeDot(const NamedDecl *D, bool Arrow) {
>> -    return make(EOP_Dot, Arrow ? 1 : 0, D);
>> -  }
>> -
>> -  unsigned makeCall(unsigned NumArgs, const NamedDecl *D) {
>> -    return make(EOP_Call, NumArgs, D);
>> -  }
>> -
>> -  // Grab the very first declaration of virtual method D
>> -  const CXXMethodDecl* getFirstVirtualDecl(const CXXMethodDecl *D) {
>> -    while (true) {
>> -      D = D->getCanonicalDecl();
>> -      CXXMethodDecl::method_iterator I = D->begin_overridden_methods(),
>> -                                     E = D->end_overridden_methods();
>> -      if (I == E)
>> -        return D;  // Method does not override anything
>> -      D = *I;      // FIXME: this does not work with multiple
>> inheritance.
>> -    }
>> -    return nullptr;
>> -  }
>> -
>> -  unsigned makeMCall(unsigned NumArgs, const CXXMethodDecl *D) {
>> -    return make(EOP_MCall, NumArgs, getFirstVirtualDecl(D));
>> -  }
>> +/// Issue a warning about an invalid lock expression
>> +static void warnInvalidLock(ThreadSafetyHandler &Handler,
>> +                            const Expr *MutexExp, const NamedDecl *D,
>> +                            const Expr *DeclExp, StringRef Kind) {
>> +  SourceLocation Loc;
>> +  if (DeclExp)
>> +    Loc = DeclExp->getExprLoc();
>>
>> -  unsigned makeIndex() {
>> -    return make(EOP_Index);
>> -  }
>> -
>> -  unsigned makeUnary() {
>> -    return make(EOP_Unary);
>> -  }
>> -
>> -  unsigned makeBinary() {
>> -    return make(EOP_Binary);
>> -  }
>> -
>> -  unsigned makeUnknown(unsigned Arity) {
>> -    return make(EOP_Unknown, Arity);
>> -  }
>> -
>> -  inline bool isCalleeArrow(const Expr *E) {
>> -    const MemberExpr *ME = dyn_cast<MemberExpr>(E->IgnoreParenCasts());
>> -    return ME ? ME->isArrow() : false;
>> -  }
>> -
>> -  /// Build an SExpr from the given C++ expression.
>> -  /// Recursive function that terminates on DeclRefExpr.
>> -  /// Note: this function merely creates a SExpr; it does not check to
>> -  /// ensure that the original expression is a valid mutex expression.
>> -  ///
>> -  /// NDeref returns the number of Derefence and AddressOf operations
>> -  /// preceding the Expr; this is used to decide whether to pretty-print
>> -  /// SExprs with . or ->.
>> -  unsigned buildSExpr(const Expr *Exp, CallingContext *CallCtx,
>> -                      int *NDeref = nullptr) {
>> -    if (!Exp)
>> -      return 0;
>> -
>> -    if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp)) {
>> -      const NamedDecl *ND =
>> cast<NamedDecl>(DRE->getDecl()->getCanonicalDecl());
>> -      const ParmVarDecl *PV = dyn_cast_or_null<ParmVarDecl>(ND);
>> -      if (PV) {
>> -        const FunctionDecl *FD =
>> -          cast<FunctionDecl>(PV->getDeclContext())->getCanonicalDecl();
>> -        unsigned i = PV->getFunctionScopeIndex();
>> -
>> -        if (CallCtx && CallCtx->FunArgs &&
>> -            FD == CallCtx->AttrDecl->getCanonicalDecl()) {
>> -          // Substitute call arguments for references to function
>> parameters
>> -          assert(i < CallCtx->NumArgs);
>> -          return buildSExpr(CallCtx->FunArgs[i], CallCtx->PrevCtx,
>> NDeref);
>> -        }
>> -        // Map the param back to the param of the original function
>> declaration.
>> -        makeNamedVar(FD->getParamDecl(i));
>> -        return 1;
>> -      }
>> -      // Not a function parameter -- just store the reference.
>> -      makeNamedVar(ND);
>> -      return 1;
>> -    } else if (isa<CXXThisExpr>(Exp)) {
>> -      // Substitute parent for 'this'
>> -      if (CallCtx && CallCtx->SelfArg) {
>> -        if (!CallCtx->SelfArrow && NDeref)
>> -          // 'this' is a pointer, but self is not, so need to take
>> address.
>> -          --(*NDeref);
>> -        return buildSExpr(CallCtx->SelfArg, CallCtx->PrevCtx, NDeref);
>> -      }
>> -      else {
>> -        makeThis();
>> -        return 1;
>> -      }
>> -    } else if (const MemberExpr *ME = dyn_cast<MemberExpr>(Exp)) {
>> -      const NamedDecl *ND = ME->getMemberDecl();
>> -      int ImplicitDeref = ME->isArrow() ? 1 : 0;
>> -      unsigned Root = makeDot(ND, false);
>> -      unsigned Sz = buildSExpr(ME->getBase(), CallCtx, &ImplicitDeref);
>> -      NodeVec[Root].setArrow(ImplicitDeref > 0);
>> -      NodeVec[Root].setSize(Sz + 1);
>> -      return Sz + 1;
>> -    } else if (const CXXMemberCallExpr *CMCE =
>> dyn_cast<CXXMemberCallExpr>(Exp)) {
>> -      // When calling a function with a lock_returned attribute, replace
>> -      // the function call with the expression in lock_returned.
>> -      const CXXMethodDecl *MD =
>> CMCE->getMethodDecl()->getMostRecentDecl();
>> -      if (LockReturnedAttr* At = MD->getAttr<LockReturnedAttr>()) {
>> -        CallingContext LRCallCtx(CMCE->getMethodDecl());
>> -        LRCallCtx.SelfArg = CMCE->getImplicitObjectArgument();
>> -        LRCallCtx.SelfArrow = isCalleeArrow(CMCE->getCallee());
>> -        LRCallCtx.NumArgs = CMCE->getNumArgs();
>> -        LRCallCtx.FunArgs = CMCE->getArgs();
>> -        LRCallCtx.PrevCtx = CallCtx;
>> -        return buildSExpr(At->getArg(), &LRCallCtx);
>> -      }
>> -      // Hack to treat smart pointers and iterators as pointers;
>> -      // ignore any method named get().
>> -      if (CMCE->getMethodDecl()->getNameAsString() == "get" &&
>> -          CMCE->getNumArgs() == 0) {
>> -        if (NDeref && isCalleeArrow(CMCE->getCallee()))
>> -          ++(*NDeref);
>> -        return buildSExpr(CMCE->getImplicitObjectArgument(), CallCtx,
>> NDeref);
>> -      }
>> -      unsigned NumCallArgs = CMCE->getNumArgs();
>> -      unsigned Root = makeMCall(NumCallArgs, CMCE->getMethodDecl());
>> -      unsigned Sz = buildSExpr(CMCE->getImplicitObjectArgument(),
>> CallCtx);
>> -      const Expr* const* CallArgs = CMCE->getArgs();
>> -      for (unsigned i = 0; i < NumCallArgs; ++i) {
>> -        Sz += buildSExpr(CallArgs[i], CallCtx);
>> -      }
>> -      NodeVec[Root].setSize(Sz + 1);
>> -      return Sz + 1;
>> -    } else if (const CallExpr *CE = dyn_cast<CallExpr>(Exp)) {
>> -      const FunctionDecl *FD =
>> CE->getDirectCallee()->getMostRecentDecl();
>> -      if (LockReturnedAttr* At = FD->getAttr<LockReturnedAttr>()) {
>> -        CallingContext LRCallCtx(CE->getDirectCallee());
>> -        LRCallCtx.NumArgs = CE->getNumArgs();
>> -        LRCallCtx.FunArgs = CE->getArgs();
>> -        LRCallCtx.PrevCtx = CallCtx;
>> -        return buildSExpr(At->getArg(), &LRCallCtx);
>> -      }
>> -      // Treat smart pointers and iterators as pointers;
>> -      // ignore the * and -> operators.
>> -      if (const CXXOperatorCallExpr *OE =
>> dyn_cast<CXXOperatorCallExpr>(CE)) {
>> -        OverloadedOperatorKind k = OE->getOperator();
>> -        if (k == OO_Star) {
>> -          if (NDeref) ++(*NDeref);
>> -          return buildSExpr(OE->getArg(0), CallCtx, NDeref);
>> -        }
>> -        else if (k == OO_Arrow) {
>> -          return buildSExpr(OE->getArg(0), CallCtx, NDeref);
>> -        }
>> -      }
>> -      unsigned NumCallArgs = CE->getNumArgs();
>> -      unsigned Root = makeCall(NumCallArgs, nullptr);
>> -      unsigned Sz = buildSExpr(CE->getCallee(), CallCtx);
>> -      const Expr* const* CallArgs = CE->getArgs();
>> -      for (unsigned i = 0; i < NumCallArgs; ++i) {
>> -        Sz += buildSExpr(CallArgs[i], CallCtx);
>> -      }
>> -      NodeVec[Root].setSize(Sz+1);
>> -      return Sz+1;
>> -    } else if (const BinaryOperator *BOE = dyn_cast<BinaryOperator>(Exp))
>> {
>> -      unsigned Root = makeBinary();
>> -      unsigned Sz = buildSExpr(BOE->getLHS(), CallCtx);
>> -      Sz += buildSExpr(BOE->getRHS(), CallCtx);
>> -      NodeVec[Root].setSize(Sz);
>> -      return Sz;
>> -    } else if (const UnaryOperator *UOE = dyn_cast<UnaryOperator>(Exp)) {
>> -      // Ignore & and * operators -- they're no-ops.
>> -      // However, we try to figure out whether the expression is a
>> pointer,
>> -      // so we can use . and -> appropriately in error messages.
>> -      if (UOE->getOpcode() == UO_Deref) {
>> -        if (NDeref) ++(*NDeref);
>> -        return buildSExpr(UOE->getSubExpr(), CallCtx, NDeref);
>> -      }
>> -      if (UOE->getOpcode() == UO_AddrOf) {
>> -        if (DeclRefExpr* DRE = dyn_cast<DeclRefExpr>(UOE->getSubExpr()))
>> {
>> -          if (DRE->getDecl()->isCXXInstanceMember()) {
>> -            // This is a pointer-to-member expression, e.g.
>> &MyClass::mu_.
>> -            // We interpret this syntax specially, as a wildcard.
>> -            unsigned Root = makeDot(DRE->getDecl(), false);
>> -            makeWildcard();
>> -            NodeVec[Root].setSize(2);
>> -            return 2;
>> -          }
>> -        }
>> -        if (NDeref) --(*NDeref);
>> -        return buildSExpr(UOE->getSubExpr(), CallCtx, NDeref);
>> -      }
>> -      unsigned Root = makeUnary();
>> -      unsigned Sz = buildSExpr(UOE->getSubExpr(), CallCtx);
>> -      NodeVec[Root].setSize(Sz);
>> -      return Sz;
>> -    } else if (const ArraySubscriptExpr *ASE =
>> -               dyn_cast<ArraySubscriptExpr>(Exp)) {
>> -      unsigned Root = makeIndex();
>> -      unsigned Sz = buildSExpr(ASE->getBase(), CallCtx);
>> -      Sz += buildSExpr(ASE->getIdx(), CallCtx);
>> -      NodeVec[Root].setSize(Sz);
>> -      return Sz;
>> -    } else if (const AbstractConditionalOperator *CE =
>> -               dyn_cast<AbstractConditionalOperator>(Exp)) {
>> -      unsigned Root = makeUnknown(3);
>> -      unsigned Sz = buildSExpr(CE->getCond(), CallCtx);
>> -      Sz += buildSExpr(CE->getTrueExpr(), CallCtx);
>> -      Sz += buildSExpr(CE->getFalseExpr(), CallCtx);
>> -      NodeVec[Root].setSize(Sz);
>> -      return Sz;
>> -    } else if (const ChooseExpr *CE = dyn_cast<ChooseExpr>(Exp)) {
>> -      unsigned Root = makeUnknown(3);
>> -      unsigned Sz = buildSExpr(CE->getCond(), CallCtx);
>> -      Sz += buildSExpr(CE->getLHS(), CallCtx);
>> -      Sz += buildSExpr(CE->getRHS(), CallCtx);
>> -      NodeVec[Root].setSize(Sz);
>> -      return Sz;
>> -    } else if (const CastExpr *CE = dyn_cast<CastExpr>(Exp)) {
>> -      return buildSExpr(CE->getSubExpr(), CallCtx, NDeref);
>> -    } else if (const ParenExpr *PE = dyn_cast<ParenExpr>(Exp)) {
>> -      return buildSExpr(PE->getSubExpr(), CallCtx, NDeref);
>> -    } else if (const ExprWithCleanups *EWC =
>> dyn_cast<ExprWithCleanups>(Exp)) {
>> -      return buildSExpr(EWC->getSubExpr(), CallCtx, NDeref);
>> -    } else if (const CXXBindTemporaryExpr *E =
>> dyn_cast<CXXBindTemporaryExpr>(Exp)) {
>> -      return buildSExpr(E->getSubExpr(), CallCtx, NDeref);
>> -    } else if (isa<CharacterLiteral>(Exp) ||
>> -               isa<CXXNullPtrLiteralExpr>(Exp) ||
>> -               isa<GNUNullExpr>(Exp) ||
>> -               isa<CXXBoolLiteralExpr>(Exp) ||
>> -               isa<FloatingLiteral>(Exp) ||
>> -               isa<ImaginaryLiteral>(Exp) ||
>> -               isa<IntegerLiteral>(Exp) ||
>> -               isa<StringLiteral>(Exp) ||
>> -               isa<ObjCStringLiteral>(Exp)) {
>> -      makeNop();
>> -      return 1;  // FIXME: Ignore literals for now
>> -    } else {
>> -      makeNop();
>> -      return 1;  // Ignore.  FIXME: mark as invalid expression?
>> -    }
>> -  }
>> -
>> -  /// \brief Construct a SExpr from an expression.
>> -  /// \param MutexExp The original mutex expression within an attribute
>> -  /// \param DeclExp An expression involving the Decl on which the
>> attribute
>> -  ///        occurs.
>> -  /// \param D  The declaration to which the lock/unlock attribute is
>> attached.
>> -  void buildSExprFromExpr(const Expr *MutexExp, const Expr *DeclExp,
>> -                          const NamedDecl *D, VarDecl *SelfDecl =
>> nullptr) {
>> -    CallingContext CallCtx(D);
>> -
>> -    if (MutexExp) {
>> -      if (const StringLiteral* SLit = dyn_cast<StringLiteral>(MutexExp))
>> {
>> -        if (SLit->getString() == StringRef("*"))
>> -          // The "*" expr is a universal lock, which essentially turns
>> off
>> -          // checks until it is removed from the lockset.
>> -          makeUniversal();
>> -        else
>> -          // Ignore other string literals for now.
>> -          makeNop();
>> -        return;
>> -      }
>> -    }
>> -
>> -    // If we are processing a raw attribute expression, with no
>> substitutions.
>> -    if (!DeclExp) {
>> -      buildSExpr(MutexExp, nullptr);
>> -      return;
>> -    }
>> -
>> -    // Examine DeclExp to find SelfArg and FunArgs, which are used to
>> substitute
>> -    // for formal parameters when we call buildMutexID later.
>> -    if (const MemberExpr *ME = dyn_cast<MemberExpr>(DeclExp)) {
>> -      CallCtx.SelfArg   = ME->getBase();
>> -      CallCtx.SelfArrow = ME->isArrow();
>> -    } else if (const CXXMemberCallExpr *CE =
>> -               dyn_cast<CXXMemberCallExpr>(DeclExp)) {
>> -      CallCtx.SelfArg   = CE->getImplicitObjectArgument();
>> -      CallCtx.SelfArrow = isCalleeArrow(CE->getCallee());
>> -      CallCtx.NumArgs   = CE->getNumArgs();
>> -      CallCtx.FunArgs   = CE->getArgs();
>> -    } else if (const CallExpr *CE = dyn_cast<CallExpr>(DeclExp)) {
>> -      CallCtx.NumArgs = CE->getNumArgs();
>> -      CallCtx.FunArgs = CE->getArgs();
>> -    } else if (const CXXConstructExpr *CE =
>> -               dyn_cast<CXXConstructExpr>(DeclExp)) {
>> -      CallCtx.SelfArg = nullptr;  // Will be set below
>> -      CallCtx.NumArgs = CE->getNumArgs();
>> -      CallCtx.FunArgs = CE->getArgs();
>> -    } else if (D && isa<CXXDestructorDecl>(D)) {
>> -      // There's no such thing as a "destructor call" in the AST.
>> -      CallCtx.SelfArg = DeclExp;
>> -    }
>> -
>> -    // Hack to handle constructors, where self cannot be recovered from
>> -    // the expression.
>> -    if (SelfDecl && !CallCtx.SelfArg) {
>> -      DeclRefExpr SelfDRE(SelfDecl, false, SelfDecl->getType(),
>> VK_LValue,
>> -                          SelfDecl->getLocation());
>> -      CallCtx.SelfArg = &SelfDRE;
>> -
>> -      // If the attribute has no arguments, then assume the argument is
>> "this".
>> -      if (!MutexExp)
>> -        buildSExpr(CallCtx.SelfArg, nullptr);
>> -      else  // For most attributes.
>> -        buildSExpr(MutexExp, &CallCtx);
>> -      return;
>> -    }
>> +  // FIXME: add a note about the attribute location in MutexExp or D
>> +  if (Loc.isValid())
>> +    Handler.handleInvalidLockExp(Kind, Loc);
>> +}
>>
>> -    // If the attribute has no arguments, then assume the argument is
>> "this".
>> -    if (!MutexExp)
>> -      buildSExpr(CallCtx.SelfArg, nullptr);
>> -    else  // For most attributes.
>> -      buildSExpr(MutexExp, &CallCtx);
>> -  }
>>
>> -  /// \brief Get index of next sibling of node i.
>> -  unsigned getNextSibling(unsigned i) const {
>> -    return i + NodeVec[i].size();
>> -  }
>> +// Various helper functions on til::SExpr
>> +namespace sx {
>>
>> -public:
>> -  explicit SExpr(clang::Decl::EmptyShell e) { NodeVec.clear(); }
>> -
>> -  /// \param MutexExp The original mutex expression within an attribute
>> -  /// \param DeclExp An expression involving the Decl on which the
>> attribute
>> -  ///        occurs.
>> -  /// \param D  The declaration to which the lock/unlock attribute is
>> attached.
>> -  /// Caller must check isValid() after construction.
>> -  SExpr(const Expr *MutexExp, const Expr *DeclExp, const NamedDecl *D,
>> -        VarDecl *SelfDecl = nullptr) {
>> -    buildSExprFromExpr(MutexExp, DeclExp, D, SelfDecl);
>> -  }
>> +bool isUniversal(const til::SExpr *E) {
>> +  return isa<til::Wildcard>(E);
>> +}
>>
>> -  /// Return true if this is a valid decl sequence.
>> -  /// Caller must call this by hand after construction to handle errors.
>> -  bool isValid() const {
>> -    return !NodeVec.empty();
>> -  }
>> +bool equals(const til::SExpr *E1, const til::SExpr *E2) {
>> +  return til::EqualsComparator::compareExprs(E1, E2);
>> +}
>>
>> -  bool shouldIgnore() const {
>> -    // Nop is a mutex that we have decided to deliberately ignore.
>> -    assert(NodeVec.size() > 0 && "Invalid Mutex");
>> -    return NodeVec[0].kind() == EOP_Nop;
>> +const til::SExpr* ignorePtrCasts(const til::SExpr *E) {
>> +  if (auto *CE = dyn_cast<til::Cast>(E)) {
>> +    if (CE->castOpcode() == til::CAST_objToPtr)
>> +      return CE->expr();
>>    }
>> +  return E;
>> +}
>>
>> -  bool isUniversal() const {
>> -    assert(NodeVec.size() > 0 && "Invalid Mutex");
>> -    return NodeVec[0].kind() == EOP_Universal;
>> -  }
>> +bool matches(const til::SExpr *E1, const til::SExpr *E2) {
>> +  // We treat a top-level wildcard as the "univsersal" lock.
>> +  // It matches everything for the purpose of checking locks, but not
>> +  // for unlocking them.
>> +  if (isa<til::Wildcard>(E1))
>> +    return isa<til::Wildcard>(E2);
>> +  if (isa<til::Wildcard>(E2))
>> +    return isa<til::Wildcard>(E1);
>>
>> -  /// Issue a warning about an invalid lock expression
>> -  static void warnInvalidLock(ThreadSafetyHandler &Handler,
>> -                              const Expr *MutexExp, const Expr *DeclExp,
>> -                              const NamedDecl *D, StringRef Kind) {
>> -    SourceLocation Loc;
>> -    if (DeclExp)
>> -      Loc = DeclExp->getExprLoc();
>> +  return til::MatchComparator::compareExprs(E1, E2);
>> +}
>>
>> -    // FIXME: add a note about the attribute location in MutexExp or D
>> -    if (Loc.isValid())
>> -      Handler.handleInvalidLockExp(Kind, Loc);
>> -  }
>> +bool partiallyMatches(const til::SExpr *E1, const til::SExpr *E2) {
>> +  auto *PE1 = dyn_cast_or_null<til::Project>(E1);
>> +  if (!PE1)
>> +    return false;
>> +  auto *PE2 = dyn_cast_or_null<til::Project>(E2);
>> +  if (!PE2)
>> +    return false;
>> +  return PE1->clangDecl() == PE2->clangDecl();
>> +}
>>
>> -  bool operator==(const SExpr &other) const {
>> -    return NodeVec == other.NodeVec;
>> -  }
>> +std::string toString(const til::SExpr *E) {
>> +  std::stringstream ss;
>> +  til::StdPrinter::print(E, ss);
>> +  return ss.str();
>> +}
>>
>> -  bool operator!=(const SExpr &other) const {
>> -    return !(*this == other);
>> -  }
>> +bool shouldIgnore(const til::SExpr *E) {
>> +  if (!E)
>> +    return true;
>> +  // Trap mutex expressions like nullptr, or 0.
>> +  // Any literal value is nonsense.
>> +  if (isa<til::Literal>(E))
>> +    return true;
>> +  return false;
>> +}
>>
>> -  bool matches(const SExpr &Other, unsigned i = 0, unsigned j = 0) const
>> {
>> -    if (NodeVec[i].matches(Other.NodeVec[j])) {
>> -      unsigned ni = NodeVec[i].arity();
>> -      unsigned nj = Other.NodeVec[j].arity();
>> -      unsigned n = (ni < nj) ? ni : nj;
>> -      bool Result = true;
>> -      unsigned ci = i+1;  // first child of i
>> -      unsigned cj = j+1;  // first child of j
>> -      for (unsigned k = 0; k < n;
>> -           ++k, ci=getNextSibling(ci), cj = Other.getNextSibling(cj)) {
>> -        Result = Result && matches(Other, ci, cj);
>> -      }
>> -      return Result;
>> -    }
>> -    return false;
>> -  }
>> +}  // end namespace sx
>>
>> -  // A partial match between a.mu and b.mu returns true a and b have the
>> same
>> -  // type (and thus mu refers to the same mutex declaration), regardless
>> of
>> -  // whether a and b are different objects or not.
>> -  bool partiallyMatches(const SExpr &Other) const {
>> -    if (NodeVec[0].kind() == EOP_Dot)
>> -      return NodeVec[0].matches(Other.NodeVec[0]);
>> -    return false;
>> -  }
>>
>> -  /// \brief Pretty print a lock expression for use in error messages.
>> -  std::string toString(unsigned i = 0) const {
>> -    assert(isValid());
>> -    if (i >= NodeVec.size())
>> -      return "";
>> -
>> -    const SExprNode* N = &NodeVec[i];
>> -    switch (N->kind()) {
>> -      case EOP_Nop:
>> -        return "_";
>> -      case EOP_Wildcard:
>> -        return "(?)";
>> -      case EOP_Universal:
>> -        return "*";
>> -      case EOP_This:
>> -        return "this";
>> -      case EOP_NVar:
>> -      case EOP_LVar: {
>> -        return N->getNamedDecl()->getNameAsString();
>> -      }
>> -      case EOP_Dot: {
>> -        if (NodeVec[i+1].kind() == EOP_Wildcard) {
>> -          std::string S = "&";
>> -          S += N->getNamedDecl()->getQualifiedNameAsString();
>> -          return S;
>> -        }
>> -        std::string FieldName = N->getNamedDecl()->getNameAsString();
>> -        if (NodeVec[i+1].kind() == EOP_This)
>> -          return FieldName;
>> -
>> -        std::string S = toString(i+1);
>> -        if (N->isArrow())
>> -          return S + "->" + FieldName;
>> -        else
>> -          return S + "." + FieldName;
>> -      }
>> -      case EOP_Call: {
>> -        std::string S = toString(i+1) + "(";
>> -        unsigned NumArgs = N->arity()-1;
>> -        unsigned ci = getNextSibling(i+1);
>> -        for (unsigned k=0; k<NumArgs; ++k, ci = getNextSibling(ci)) {
>> -          S += toString(ci);
>> -          if (k+1 < NumArgs) S += ",";
>> -        }
>> -        S += ")";
>> -        return S;
>> -      }
>> -      case EOP_MCall: {
>> -        std::string S = "";
>> -        if (NodeVec[i+1].kind() != EOP_This)
>> -          S = toString(i+1) + ".";
>> -        if (const NamedDecl *D = N->getFunctionDecl())
>> -          S += D->getNameAsString() + "(";
>> -        else
>> -          S += "#(";
>> -        unsigned NumArgs = N->arity()-1;
>> -        unsigned ci = getNextSibling(i+1);
>> -        for (unsigned k=0; k<NumArgs; ++k, ci = getNextSibling(ci)) {
>> -          S += toString(ci);
>> -          if (k+1 < NumArgs) S += ",";
>> -        }
>> -        S += ")";
>> -        return S;
>> -      }
>> -      case EOP_Index: {
>> -        std::string S1 = toString(i+1);
>> -        std::string S2 = toString(i+1 + NodeVec[i+1].size());
>> -        return S1 + "[" + S2 + "]";
>> -      }
>> -      case EOP_Unary: {
>> -        std::string S = toString(i+1);
>> -        return "#" + S;
>> -      }
>> -      case EOP_Binary: {
>> -        std::string S1 = toString(i+1);
>> -        std::string S2 = toString(i+1 + NodeVec[i+1].size());
>> -        return "(" + S1 + "#" + S2 + ")";
>> -      }
>> -      case EOP_Unknown: {
>> -        unsigned NumChildren = N->arity();
>> -        if (NumChildren == 0)
>> -          return "(...)";
>> -        std::string S = "(";
>> -        unsigned ci = i+1;
>> -        for (unsigned j = 0; j < NumChildren; ++j, ci =
>> getNextSibling(ci)) {
>> -          S += toString(ci);
>> -          if (j+1 < NumChildren) S += "#";
>> -        }
>> -        S += ")";
>> -        return S;
>> -      }
>> -    }
>> -    return "";
>> -  }
>> -};
>>
>>  /// \brief A short list of SExprs
>> -class MutexIDList : public SmallVector<SExpr, 3> {
>> +class MutexIDList : public SmallVector<const til::SExpr*, 3> {
>>  public:
>>    /// \brief Push M onto list, but discard duplicates.
>> -  void push_back_nodup(const SExpr& M) {
>> -    if (end() == std::find(begin(), end(), M))
>> -      push_back(M);
>> +  void push_back_nodup(const til::SExpr *E) {
>> +    iterator It = std::find_if(begin(), end(), [=](const til::SExpr *E2)
>> {
>> +      return sx::equals(E, E2);
>> +    });
>> +    if (It == end())
>> +      push_back(E);
>>    }
>>  };
>>
>> @@ -735,15 +160,15 @@ struct LockData {
>>    LockKind LKind;
>>    bool     Asserted;           // for asserted locks
>>    bool     Managed;            // for ScopedLockable objects
>> -  SExpr    UnderlyingMutex;    // for ScopedLockable objects
>> +  const til::SExpr* UnderlyingMutex;    // for ScopedLockable objects
>>
>>    LockData(SourceLocation AcquireLoc, LockKind LKind, bool M=false,
>>             bool Asrt=false)
>>      : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(Asrt), Managed(M),
>> -      UnderlyingMutex(Decl::EmptyShell())
>> +      UnderlyingMutex(nullptr)
>>    {}
>>
>> -  LockData(SourceLocation AcquireLoc, LockKind LKind, const SExpr &Mu)
>> +  LockData(SourceLocation AcquireLoc, LockKind LKind, const til::SExpr
>> *Mu)
>>      : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(false),
>> Managed(false),
>>        UnderlyingMutex(Mu)
>>    {}
>> @@ -771,10 +196,10 @@ struct LockData {
>>  /// in the program execution.  Currently, this is information regarding a
>> lock
>>  /// that is held at that point.
>>  struct FactEntry {
>> -  SExpr    MutID;
>> +  const til::SExpr *MutID;
>>    LockData LDat;
>>
>> -  FactEntry(const SExpr& M, const LockData& L)
>> +  FactEntry(const til::SExpr* M, const LockData& L)
>>      : MutID(M), LDat(L)
>>    { }
>>  };
>> @@ -789,8 +214,8 @@ private:
>>    std::vector<FactEntry> Facts;
>>
>>  public:
>> -  FactID newLock(const SExpr& M, const LockData& L) {
>> -    Facts.push_back(FactEntry(M,L));
>> +  FactID newLock(const til::SExpr *M, const LockData& L) {
>> +    Facts.push_back(FactEntry(M, L));
>>      return static_cast<unsigned short>(Facts.size() - 1);
>>    }
>>
>> @@ -824,66 +249,67 @@ public:
>>
>>    bool isEmpty() const { return FactIDs.size() == 0; }
>>
>> -  FactID addLock(FactManager& FM, const SExpr& M, const LockData& L) {
>> +  FactID addLock(FactManager& FM, const til::SExpr *M, const LockData& L)
>> {
>>      FactID F = FM.newLock(M, L);
>>      FactIDs.push_back(F);
>>      return F;
>>    }
>>
>> -  bool removeLock(FactManager& FM, const SExpr& M) {
>> +  bool removeLock(FactManager& FM, const til::SExpr *M) {
>>      unsigned n = FactIDs.size();
>>      if (n == 0)
>>        return false;
>>
>>      for (unsigned i = 0; i < n-1; ++i) {
>> -      if (FM[FactIDs[i]].MutID.matches(M)) {
>> +      if (sx::matches(FM[FactIDs[i]].MutID, M)) {
>>          FactIDs[i] = FactIDs[n-1];
>>          FactIDs.pop_back();
>>          return true;
>>        }
>>      }
>> -    if (FM[FactIDs[n-1]].MutID.matches(M)) {
>> +    if (sx::matches(FM[FactIDs[n-1]].MutID, M)) {
>>        FactIDs.pop_back();
>>        return true;
>>      }
>>      return false;
>>    }
>>
>> -  iterator findLockIter(FactManager &FM, const SExpr &M) {
>> +  iterator findLockIter(FactManager &FM, const til::SExpr *M) {
>>      return std::find_if(begin(), end(), [&](FactID ID) {
>> -      return FM[ID].MutID.matches(M);
>> +      return sx::matches(FM[ID].MutID, M);
>>      });
>>    }
>>
>> -  LockData *findLock(FactManager &FM, const SExpr &M) const {
>> +  LockData *findLock(FactManager &FM, const til::SExpr *M) const {
>>      auto I = std::find_if(begin(), end(), [&](FactID ID) {
>> -      return FM[ID].MutID.matches(M);
>> +      return sx::matches(FM[ID].MutID, M);
>>      });
>>
>>      return I != end() ? &FM[*I].LDat : nullptr;
>>    }
>>
>> -  LockData *findLockUniv(FactManager &FM, const SExpr &M) const {
>> +  LockData *findLockUniv(FactManager &FM, const til::SExpr *M) const {
>>      auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool {
>> -      const SExpr &Expr = FM[ID].MutID;
>> -      return Expr.isUniversal() || Expr.matches(M);
>> +      const til::SExpr *E = FM[ID].MutID;
>> +      return sx::isUniversal(E) || sx::matches(E, M);
>>      });
>>
>>      return I != end() ? &FM[*I].LDat : nullptr;
>>    }
>>
>> -  FactEntry *findPartialMatch(FactManager &FM, const SExpr &M) const {
>> +  FactEntry *findPartialMatch(FactManager &FM, const til::SExpr *M) const
>> {
>>      auto I = std::find_if(begin(), end(), [&](FactID ID) {
>> -      return FM[ID].MutID.partiallyMatches(M);
>> +      return sx::partiallyMatches(FM[ID].MutID, M);
>>      });
>>
>>      return I != end() ? &FM[*I] : nullptr;
>>    }
>>  };
>>
>> +
>>  /// A Lockset maps each SExpr (defined above) to information about how it
>> has
>>  /// been locked.
>> -typedef llvm::ImmutableMap<SExpr, LockData> Lockset;
>> +typedef llvm::ImmutableMap<til::SExpr*, LockData> Lockset;
>>  typedef llvm::ImmutableMap<const NamedDecl*, unsigned> LocalVarContext;
>>
>>  class LocalVariableMap;
>> @@ -1408,18 +834,24 @@ static void findBlockLocations(CFG *CFGr
>>  class ThreadSafetyAnalyzer {
>>    friend class BuildLockset;
>>
>> +  llvm::BumpPtrAllocator Bpa;
>> +  threadSafety::til::MemRegionRef Arena;
>> +  threadSafety::SExprBuilder SxBuilder;
>> +
>>    ThreadSafetyHandler       &Handler;
>>    LocalVariableMap          LocalVarMap;
>>    FactManager               FactMan;
>>    std::vector<CFGBlockInfo> BlockInfo;
>>
>>  public:
>> -  ThreadSafetyAnalyzer(ThreadSafetyHandler &H) : Handler(H) {}
>> +  ThreadSafetyAnalyzer(ThreadSafetyHandler &H)
>> +     : Arena(&Bpa), SxBuilder(Arena), Handler(H) {}
>>
>> -  void addLock(FactSet &FSet, const SExpr &Mutex, const LockData &LDat,
>> +  void addLock(FactSet &FSet, const til::SExpr *Mutex, const LockData
>> &LDat,
>>                 StringRef DiagKind);
>> -  void removeLock(FactSet &FSet, const SExpr &Mutex, SourceLocation
>> UnlockLoc,
>> -                  bool FullyRemove, LockKind Kind, StringRef DiagKind);
>> +  void removeLock(FactSet &FSet, const til::SExpr *Mutex,
>> +                  SourceLocation UnlockLoc, bool FullyRemove, LockKind
>> Kind,
>> +                  StringRef DiagKind);
>>
>>    template <typename AttrType>
>>    void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp,
>> @@ -1533,16 +965,16 @@ ClassifyDiagnostic(const AttrTy *A) {
>>  /// \brief Add a new lock to the lockset, warning if the lock is already
>> there.
>>  /// \param Mutex -- the Mutex expression for the lock
>>  /// \param LDat  -- the LockData for the lock
>> -void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const SExpr &Mutex,
>> +void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const til::SExpr
>> *Mutex,
>>                                     const LockData &LDat, StringRef
>> DiagKind) {
>>    // FIXME: deal with acquired before/after annotations.
>>    // FIXME: Don't always warn when we have support for reentrant locks.
>> -  if (Mutex.shouldIgnore())
>> +  if (sx::shouldIgnore(Mutex))
>>      return;
>>
>>    if (FSet.findLock(FactMan, Mutex)) {
>>      if (!LDat.Asserted)
>> -      Handler.handleDoubleLock(DiagKind, Mutex.toString(),
>> LDat.AcquireLoc);
>> +      Handler.handleDoubleLock(DiagKind, sx::toString(Mutex),
>> LDat.AcquireLoc);
>>    } else {
>>      FSet.addLock(FactMan, Mutex, LDat);
>>    }
>> @@ -1552,28 +984,28 @@ void ThreadSafetyAnalyzer::addLock(FactS
>>  /// \brief Remove a lock from the lockset, warning if the lock is not
>> there.
>>  /// \param Mutex The lock expression corresponding to the lock to be
>> removed
>>  /// \param UnlockLoc The source location of the unlock (only used in
>> error msg)
>> -void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const SExpr &Mutex,
>> +void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const til::SExpr
>> *Mutex,
>>                                        SourceLocation UnlockLoc,
>>                                        bool FullyRemove, LockKind
>> ReceivedKind,
>>                                        StringRef DiagKind) {
>> -  if (Mutex.shouldIgnore())
>> +  if (sx::shouldIgnore(Mutex))
>>      return;
>>
>>    const LockData *LDat = FSet.findLock(FactMan, Mutex);
>>    if (!LDat) {
>> -    Handler.handleUnmatchedUnlock(DiagKind, Mutex.toString(), UnlockLoc);
>> +    Handler.handleUnmatchedUnlock(DiagKind, sx::toString(Mutex),
>> UnlockLoc);
>>      return;
>>    }
>>
>>    // Generic lock removal doesn't care about lock kind mismatches, but
>>    // otherwise diagnose when the lock kinds are mismatched.
>>    if (ReceivedKind != LK_Generic && LDat->LKind != ReceivedKind) {
>> -    Handler.handleIncorrectUnlockKind(DiagKind, Mutex.toString(),
>> LDat->LKind,
>> -                                      ReceivedKind, UnlockLoc);
>> +    Handler.handleIncorrectUnlockKind(DiagKind, sx::toString(Mutex),
>> +                                      LDat->LKind, ReceivedKind,
>> UnlockLoc);
>>      return;
>>    }
>>
>> -  if (LDat->UnderlyingMutex.isValid()) {
>> +  if (LDat->UnderlyingMutex) {
>>      // This is scoped lockable object, which manages the real mutex.
>>      if (FullyRemove) {
>>        // We're destroying the managing object.
>> @@ -1585,7 +1017,7 @@ void ThreadSafetyAnalyzer::removeLock(Fa
>>        // managing object.  Warn on dual release.
>>        if (!FSet.findLock(FactMan, LDat->UnderlyingMutex)) {
>>          Handler.handleUnmatchedUnlock(
>> -            DiagKind, LDat->UnderlyingMutex.toString(), UnlockLoc);
>> +            DiagKind, sx::toString(LDat->UnderlyingMutex), UnlockLoc);
>>        }
>>        FSet.removeLock(FactMan, LDat->UnderlyingMutex);
>>        return;
>> @@ -1603,20 +1035,25 @@ void ThreadSafetyAnalyzer::getMutexIDs(M
>>                                         VarDecl *SelfDecl) {
>>    if (Attr->args_size() == 0) {
>>      // The mutex held is the "this" object.
>> -    SExpr Mu(nullptr, Exp, D, SelfDecl);
>> -    if (!Mu.isValid())
>> -      SExpr::warnInvalidLock(Handler, nullptr, Exp, D,
>> -                             ClassifyDiagnostic(Attr));
>> -    else
>> +    til::SExpr *Mu = SxBuilder.translateAttrExpr(nullptr, D, Exp,
>> SelfDecl);
>> +    if (Mu && isa<til::Undefined>(Mu)) {
>> +       warnInvalidLock(Handler, nullptr, D, Exp,
>> ClassifyDiagnostic(Attr));
>> +       return;
>> +    }
>> +    //else
>> +    if (Mu)
>>        Mtxs.push_back_nodup(Mu);
>>      return;
>>    }
>>
>>    for (const auto *Arg : Attr->args()) {
>> -    SExpr Mu(Arg, Exp, D, SelfDecl);
>> -    if (!Mu.isValid())
>> -      SExpr::warnInvalidLock(Handler, Arg, Exp, D,
>> ClassifyDiagnostic(Attr));
>> -    else
>> +    til::SExpr *Mu = SxBuilder.translateAttrExpr(Arg, D, Exp, SelfDecl);
>> +    if (Mu && isa<til::Undefined>(Mu)) {
>> +       warnInvalidLock(Handler, nullptr, D, Exp,
>> ClassifyDiagnostic(Attr));
>> +       return;
>> +    }
>> +    //else
>> +    if (Mu)
>>        Mtxs.push_back_nodup(Mu);
>>    }
>>  }
>> @@ -1845,11 +1282,12 @@ void BuildLockset::warnIfMutexNotHeld(co
>>                                        StringRef DiagKind) {
>>    LockKind LK = getLockKindFromAccessKind(AK);
>>
>> -  SExpr Mutex(MutexExp, Exp, D);
>> -  if (!Mutex.isValid()) {
>> -    SExpr::warnInvalidLock(Analyzer->Handler, MutexExp, Exp, D,
>> DiagKind);
>> +  til::SExpr *Mutex = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D,
>> Exp);
>> +  if (!Mutex) {
>> +    // TODO: invalid locks?
>> +    // warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind);
>>      return;
>> -  } else if (Mutex.shouldIgnore()) {
>> +  } else if (sx::shouldIgnore(Mutex)) {
>>      return;
>>    }
>>
>> @@ -1861,38 +1299,42 @@ void BuildLockset::warnIfMutexNotHeld(co
>>      if (FEntry) {
>>        // Warn that there's no precise match.
>>        LDat = &FEntry->LDat;
>> -      std::string PartMatchStr = FEntry->MutID.toString();
>> +      std::string PartMatchStr = sx::toString(FEntry->MutID);
>>        StringRef   PartMatchName(PartMatchStr);
>> -      Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
>> Mutex.toString(),
>> +      Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
>> +                                           sx::toString(Mutex),
>>                                             LK, Exp->getExprLoc(),
>>                                             &PartMatchName);
>>      } else {
>>        // Warn that there's no match at all.
>> -      Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
>> Mutex.toString(),
>> +      Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
>> +                                           sx::toString(Mutex),
>>                                             LK, Exp->getExprLoc());
>>      }
>>      NoError = false;
>>    }
>>    // Make sure the mutex we found is the right kind.
>>    if (NoError && LDat && !LDat->isAtLeast(LK))
>> -    Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
>> Mutex.toString(), LK,
>> -                                         Exp->getExprLoc());
>> +    Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK,
>> +                                         sx::toString(Mutex),
>> +                                         LK, Exp->getExprLoc());
>>  }
>>
>>  /// \brief Warn if the LSet contains the given lock.
>>  void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp,
>>                                     Expr *MutexExp,
>>                                     StringRef DiagKind) {
>> -  SExpr Mutex(MutexExp, Exp, D);
>> -  if (!Mutex.isValid()) {
>> -    SExpr::warnInvalidLock(Analyzer->Handler, MutexExp, Exp, D,
>> DiagKind);
>> +  til::SExpr *Mutex = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D,
>> Exp);
>> +  if (!Mutex) {
>> +    // TODO: invalid locks?
>> +    // warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind);
>>      return;
>>    }
>>
>>    LockData* LDat = FSet.findLock(Analyzer->FactMan, Mutex);
>>    if (LDat)
>>      Analyzer->Handler.handleFunExcludesLock(
>> -        DiagKind, D->getNameAsString(), Mutex.toString(),
>> Exp->getExprLoc());
>> +        DiagKind, D->getNameAsString(), sx::toString(Mutex),
>> Exp->getExprLoc());
>>  }
>>
>>  /// \brief Checks guarded_by and pt_guarded_by attributes.
>> @@ -2085,7 +1527,7 @@ void BuildLockset::handleCall(Expr *Exp,
>>    if (isScopedVar) {
>>      SourceLocation MLoc = VD->getLocation();
>>      DeclRefExpr DRE(VD, false, VD->getType(), VK_LValue,
>> VD->getLocation());
>> -    SExpr SMutex(&DRE, nullptr, nullptr);
>> +    til::SExpr *SMutex = Analyzer->SxBuilder.translateAttrExpr(&DRE,
>> nullptr);
>>
>>      for (const auto &M : ExclusiveLocksToAdd)
>>        Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive, M),
>> @@ -2093,6 +1535,12 @@ void BuildLockset::handleCall(Expr *Exp,
>>      for (const auto &M : SharedLocksToAdd)
>>        Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Shared, M),
>>                          CapDiagKind);
>> +
>> +    // handle corner case where the underlying mutex is invalid
>> +    if (ExclusiveLocksToAdd.size() == 0 && SharedLocksToAdd.size() == 0)
>> {
>> +      Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive),
>> +                        CapDiagKind);
>> +    }
>>    }
>>
>>    // Remove locks.
>> @@ -2254,14 +1702,14 @@ void ThreadSafetyAnalyzer::intersectAndW
>>
>>    // Find locks in FSet2 that conflict or are not in FSet1, and warn.
>>    for (const auto &Fact : FSet2) {
>> -    const SExpr &FSet2Mutex = FactMan[Fact].MutID;
>> +    const til::SExpr *FSet2Mutex = FactMan[Fact].MutID;
>>      const LockData &LDat2 = FactMan[Fact].LDat;
>>      FactSet::iterator I1 = FSet1.findLockIter(FactMan, FSet2Mutex);
>>
>>      if (I1 != FSet1.end()) {
>>        const LockData* LDat1 = &FactMan[*I1].LDat;
>>        if (LDat1->LKind != LDat2.LKind) {
>> -        Handler.handleExclusiveAndShared("mutex", FSet2Mutex.toString(),
>> +        Handler.handleExclusiveAndShared("mutex",
>> sx::toString(FSet2Mutex),
>>                                           LDat2.AcquireLoc,
>> LDat1->AcquireLoc);
>>          if (Modify && LDat1->LKind != LK_Exclusive) {
>>            // Take the exclusive lock, which is the one in FSet2.
>> @@ -2273,40 +1721,42 @@ void ThreadSafetyAnalyzer::intersectAndW
>>          *I1 = Fact;
>>        }
>>      } else {
>> -      if (LDat2.UnderlyingMutex.isValid()) {
>> +      if (LDat2.UnderlyingMutex) {
>>          if (FSet2.findLock(FactMan, LDat2.UnderlyingMutex)) {
>>            // If this is a scoped lock that manages another mutex, and if
>> the
>>            // underlying mutex is still held, then warn about the
>> underlying
>>            // mutex.
>>            Handler.handleMutexHeldEndOfScope("mutex",
>> -
>> LDat2.UnderlyingMutex.toString(),
>> +
>> sx::toString(LDat2.UnderlyingMutex),
>>                                              LDat2.AcquireLoc, JoinLoc,
>> LEK1);
>>          }
>>        }
>> -      else if (!LDat2.Managed && !FSet2Mutex.isUniversal() &&
>> !LDat2.Asserted)
>> -        Handler.handleMutexHeldEndOfScope("mutex", FSet2Mutex.toString(),
>> +      else if (!LDat2.Managed && !sx::isUniversal(FSet2Mutex) &&
>> +               !LDat2.Asserted)
>> +        Handler.handleMutexHeldEndOfScope("mutex",
>> sx::toString(FSet2Mutex),
>>                                            LDat2.AcquireLoc, JoinLoc,
>> LEK1);
>>      }
>>    }
>>
>>    // Find locks in FSet1 that are not in FSet2, and remove them.
>>    for (const auto &Fact : FSet1Orig) {
>> -    const SExpr &FSet1Mutex = FactMan[Fact].MutID;
>> +    const til::SExpr *FSet1Mutex = FactMan[Fact].MutID;
>>      const LockData &LDat1 = FactMan[Fact].LDat;
>>
>>      if (!FSet2.findLock(FactMan, FSet1Mutex)) {
>> -      if (LDat1.UnderlyingMutex.isValid()) {
>> +      if (LDat1.UnderlyingMutex) {
>>          if (FSet1Orig.findLock(FactMan, LDat1.UnderlyingMutex)) {
>>            // If this is a scoped lock that manages another mutex, and if
>> the
>>            // underlying mutex is still held, then warn about the
>> underlying
>>            // mutex.
>>            Handler.handleMutexHeldEndOfScope("mutex",
>> -
>> LDat1.UnderlyingMutex.toString(),
>> +
>> sx::toString(LDat1.UnderlyingMutex),
>>                                              LDat1.AcquireLoc, JoinLoc,
>> LEK1);
>>          }
>>        }
>> -      else if (!LDat1.Managed && !FSet1Mutex.isUniversal() &&
>> !LDat1.Asserted)
>> -        Handler.handleMutexHeldEndOfScope("mutex", FSet1Mutex.toString(),
>> +      else if (!LDat1.Managed && !sx::isUniversal(FSet1Mutex) &&
>> +               !LDat1.Asserted)
>> +        Handler.handleMutexHeldEndOfScope("mutex",
>> sx::toString(FSet1Mutex),
>>                                            LDat1.AcquireLoc, JoinLoc,
>> LEK2);
>>        if (Modify)
>>          FSet1.removeLock(FactMan, FSet1Mutex);
>> @@ -2618,11 +2068,6 @@ void ThreadSafetyAnalyzer::runAnalysis(A
>>                     false);
>>  }
>>
>> -} // end anonymous namespace
>> -
>> -
>> -namespace clang {
>> -namespace thread_safety {
>>
>>  /// \brief Check a function's CFG for thread-safety violations.
>>  ///
>> @@ -2647,4 +2092,4 @@ LockKind getLockKindFromAccessKind(Acces
>>    llvm_unreachable("Unknown AccessKind");
>>  }
>>
>> -}} // end namespace clang::thread_safety
>> +}} // end namespace clang::threadSafety
>>
>> Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
>> +++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Mon Jul 28 10:57:27 2014
>> @@ -91,6 +91,102 @@ til::SCFG *SExprBuilder::buildCFG(CFGWal
>>  }
>>
>>
>> +
>> +inline bool isCalleeArrow(const Expr *E) {
>> +  const MemberExpr *ME = dyn_cast<MemberExpr>(E->IgnoreParenCasts());
>> +  return ME ? ME->isArrow() : false;
>> +}
>> +
>> +
>> +/// \brief Translate a clang expression in an attribute to a til::SExpr.
>> +/// Constructs the context from D, DeclExp, and SelfDecl.
>> +///
>> +/// \param AttrExp The expression to translate.
>> +/// \param D       The declaration to which the attribute is attached.
>> +/// \param DeclExp An expression involving the Decl to which the
>> attribute
>> +///                is attached.  E.g. the call to a function.
>> +til::SExpr *SExprBuilder::translateAttrExpr(const Expr *AttrExp,
>> +                                            const NamedDecl *D,
>> +                                            const Expr *DeclExp,
>> +                                            VarDecl *SelfDecl) {
>> +  // If we are processing a raw attribute expression, with no
>> substitutions.
>> +  if (!DeclExp)
>> +    return translateAttrExpr(AttrExp, nullptr);
>> +
>> +  CallingContext Ctx(nullptr, D);
>> +
>> +  // Examine DeclExp to find SelfArg and FunArgs, which are used to
>> substitute
>> +  // for formal parameters when we call buildMutexID later.
>> +  if (const MemberExpr *ME = dyn_cast<MemberExpr>(DeclExp)) {
>> +    Ctx.SelfArg   = ME->getBase();
>> +    Ctx.SelfArrow = ME->isArrow();
>> +  } else if (const CXXMemberCallExpr *CE =
>> +             dyn_cast<CXXMemberCallExpr>(DeclExp)) {
>> +    Ctx.SelfArg   = CE->getImplicitObjectArgument();
>> +    Ctx.SelfArrow = isCalleeArrow(CE->getCallee());
>> +    Ctx.NumArgs   = CE->getNumArgs();
>> +    Ctx.FunArgs   = CE->getArgs();
>> +  } else if (const CallExpr *CE = dyn_cast<CallExpr>(DeclExp)) {
>> +    Ctx.NumArgs = CE->getNumArgs();
>> +    Ctx.FunArgs = CE->getArgs();
>> +  } else if (const CXXConstructExpr *CE =
>> +             dyn_cast<CXXConstructExpr>(DeclExp)) {
>> +    Ctx.SelfArg = nullptr;  // Will be set below
>> +    Ctx.NumArgs = CE->getNumArgs();
>> +    Ctx.FunArgs = CE->getArgs();
>> +  } else if (D && isa<CXXDestructorDecl>(D)) {
>> +    // There's no such thing as a "destructor call" in the AST.
>> +    Ctx.SelfArg = DeclExp;
>> +  }
>> +
>> +  // Hack to handle constructors, where self cannot be recovered from
>> +  // the expression.
>> +  if (SelfDecl && !Ctx.SelfArg) {
>> +    DeclRefExpr SelfDRE(SelfDecl, false, SelfDecl->getType(), VK_LValue,
>> +                        SelfDecl->getLocation());
>> +    Ctx.SelfArg = &SelfDRE;
>> +
>> +    // If the attribute has no arguments, then assume the argument is
>> "this".
>> +    if (!AttrExp)
>> +      return translateAttrExpr(Ctx.SelfArg, nullptr);
>> +    else  // For most attributes.
>> +      return translateAttrExpr(AttrExp, &Ctx);
>> +  }
>> +
>> +  // If the attribute has no arguments, then assume the argument is
>> "this".
>> +  if (!AttrExp)
>> +    return translateAttrExpr(Ctx.SelfArg, nullptr);
>> +  else  // For most attributes.
>> +    return translateAttrExpr(AttrExp, &Ctx);
>> +}
>> +
>> +
>> +/// \brief Translate a clang expression in an attribute to a til::SExpr.
>> +// This assumes a CallingContext has already been created.
>> +til::SExpr *SExprBuilder::translateAttrExpr(const Expr *AttrExp,
>> +                                            CallingContext *Ctx) {
>> +  if (const StringLiteral* SLit =
>> dyn_cast_or_null<StringLiteral>(AttrExp)) {
>> +    if (SLit->getString() == StringRef("*"))
>> +      // The "*" expr is a universal lock, which essentially turns off
>> +      // checks until it is removed from the lockset.
>> +      return new (Arena) til::Wildcard();
>> +    else
>> +      // Ignore other string literals for now.
>> +      return nullptr;
>> +  }
>> +
>> +  til::SExpr *E = translate(AttrExp, Ctx);
>> +
>> +  // Hack to deal with smart pointers -- strip off top-level pointer
>> casts.
>> +  if (auto *CE = dyn_cast_or_null<til::Cast>(E)) {
>> +    if (CE->castOpcode() == til::CAST_objToPtr)
>> +      return CE->expr();
>> +  }
>> +  return E;
>> +}
>> +
>> +
>> +
>>  // Translate a clang statement or expression to a TIL expression.
>>  // Also performs substitution of variables; Ctx provides the context.
>>  // Dispatches on the type of S.
>> @@ -125,9 +221,10 @@ til::SExpr *SExprBuilder::translate(cons
>>    case Stmt::ArraySubscriptExprClass:
>>      return translateArraySubscriptExpr(cast<ArraySubscriptExpr>(S), Ctx);
>>    case Stmt::ConditionalOperatorClass:
>> -    return translateConditionalOperator(cast<ConditionalOperator>(S),
>> Ctx);
>> +    return translateAbstractConditionalOperator(
>> +             cast<ConditionalOperator>(S), Ctx);
>>    case Stmt::BinaryConditionalOperatorClass:
>> -    return translateBinaryConditionalOperator(
>> +    return translateAbstractConditionalOperator(
>>               cast<BinaryConditionalOperator>(S), Ctx);
>>
>>    // We treat these as no-ops
>> @@ -162,6 +259,7 @@ til::SExpr *SExprBuilder::translate(cons
>>  }
>>
>>
>> +
>>  til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE,
>>                                                 CallingContext *Ctx) {
>>    const ValueDecl *VD =
>> cast<ValueDecl>(DRE->getDecl()->getCanonicalDecl());
>> @@ -197,17 +295,72 @@ til::SExpr *SExprBuilder::translateCXXTh
>>  }
>>
>>
>> +const ValueDecl *getValueDeclFromSExpr(const til::SExpr *E) {
>> +  if (auto *V = dyn_cast<til::Variable>(E))
>> +    return V->clangDecl();
>> +  if (auto *P = dyn_cast<til::Project>(E))
>> +    return P->clangDecl();
>> +  if (auto *L = dyn_cast<til::LiteralPtr>(E))
>> +    return L->clangDecl();
>> +  return 0;
>> +}
>> +
>> +bool hasCppPointerType(const til::SExpr *E) {
>> +  auto *VD = getValueDeclFromSExpr(E);
>> +  if (VD && VD->getType()->isPointerType())
>> +    return true;
>> +  if (auto *C = dyn_cast<til::Cast>(E))
>> +    return C->castOpcode() == til::CAST_objToPtr;
>> +
>> +  return false;
>> +}
>> +
>> +
>> +// Grab the very first declaration of virtual method D
>> +const CXXMethodDecl* getFirstVirtualDecl(const CXXMethodDecl *D) {
>> +  while (true) {
>> +    D = D->getCanonicalDecl();
>> +    CXXMethodDecl::method_iterator I = D->begin_overridden_methods(),
>> +                                   E = D->end_overridden_methods();
>> +    if (I == E)
>> +      return D;  // Method does not override anything
>> +    D = *I;      // FIXME: this does not work with multiple inheritance.
>> +  }
>> +  return nullptr;
>> +}
>> +
>>  til::SExpr *SExprBuilder::translateMemberExpr(const MemberExpr *ME,
>>                                                CallingContext *Ctx) {
>> -  til::SExpr *E = translate(ME->getBase(), Ctx);
>> -  E = new (Arena) til::SApply(E);
>> -  return new (Arena) til::Project(E, ME->getMemberDecl());
>> +  til::SExpr *BE = translate(ME->getBase(), Ctx);
>> +  til::SExpr *E  = new (Arena) til::SApply(BE);
>> +
>> +  const ValueDecl *D = ME->getMemberDecl();
>> +  if (auto *VD = dyn_cast<CXXMethodDecl>(D))
>> +    D = getFirstVirtualDecl(VD);
>> +
>> +  til::Project *P = new (Arena) til::Project(E, D);
>> +  if (hasCppPointerType(BE))
>> +    P->setArrow(true);
>> +  return P;
>>  }
>>
>>
>>  til::SExpr *SExprBuilder::translateCallExpr(const CallExpr *CE,
>> -                                            CallingContext *Ctx) {
>> -  // TODO -- Lock returned
>> +                                            CallingContext *Ctx,
>> +                                            const Expr *SelfE) {
>> +  if (CapabilityExprMode) {
>> +    // Handle LOCK_RETURNED
>> +    const FunctionDecl *FD = CE->getDirectCallee()->getMostRecentDecl();
>> +    if (LockReturnedAttr* At = FD->getAttr<LockReturnedAttr>()) {
>> +      CallingContext LRCallCtx(Ctx);
>> +      LRCallCtx.AttrDecl = CE->getDirectCallee();
>> +      LRCallCtx.SelfArg  = SelfE;
>> +      LRCallCtx.NumArgs  = CE->getNumArgs();
>> +      LRCallCtx.FunArgs  = CE->getArgs();
>> +      return translateAttrExpr(At->getArg(), &LRCallCtx);
>> +    }
>> +  }
>> +
>>    til::SExpr *E = translate(CE->getCallee(), Ctx);
>>    for (const auto *Arg : CE->arguments()) {
>>      til::SExpr *A = translate(Arg, Ctx);
>> @@ -219,12 +372,31 @@ til::SExpr *SExprBuilder::translateCallE
>>
>>  til::SExpr *SExprBuilder::translateCXXMemberCallExpr(
>>      const CXXMemberCallExpr *ME, CallingContext *Ctx) {
>> -  return translateCallExpr(cast<CallExpr>(ME), Ctx);
>> +  if (CapabilityExprMode) {
>> +    // Ignore calls to get() on smart pointers.
>> +    if (ME->getMethodDecl()->getNameAsString() == "get" &&
>> +        ME->getNumArgs() == 0) {
>> +      auto *E = translate(ME->getImplicitObjectArgument(), Ctx);
>> +      return new (Arena) til::Cast(til::CAST_objToPtr, E);
>> +      // return E;
>> +    }
>> +  }
>> +  return translateCallExpr(cast<CallExpr>(ME), Ctx,
>> +                           ME->getImplicitObjectArgument());
>>  }
>>
>>
>>  til::SExpr *SExprBuilder::translateCXXOperatorCallExpr(
>>      const CXXOperatorCallExpr *OCE, CallingContext *Ctx) {
>> +  if (CapabilityExprMode) {
>> +    // Ignore operator * and operator -> on smart pointers.
>> +    OverloadedOperatorKind k = OCE->getOperator();
>> +    if (k == OO_Star || k == OO_Arrow) {
>> +      auto *E = translate(OCE->getArg(0), Ctx);
>> +      return new (Arena) til::Cast(til::CAST_objToPtr, E);
>> +      // return E;
>> +    }
>> +  }
>>    return translateCallExpr(cast<CallExpr>(OCE), Ctx);
>>  }
>>
>> @@ -238,8 +410,23 @@ til::SExpr *SExprBuilder::translateUnary
>>    case UO_PreDec:
>>      return new (Arena) til::Undefined(UO);
>>
>> +  case UO_AddrOf: {
>> +    if (CapabilityExprMode) {
>> +      // interpret &Graph::mu_ as an existential.
>> +      if (DeclRefExpr* DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr())) {
>> +        if (DRE->getDecl()->isCXXInstanceMember()) {
>> +          // This is a pointer-to-member expression, e.g. &MyClass::mu_.
>> +          // We interpret this syntax specially, as a wildcard.
>> +          auto *W = new (Arena) til::Wildcard();
>> +          return new (Arena) til::Project(W, DRE->getDecl());
>> +        }
>> +      }
>> +    }
>> +    // otherwise, & is a no-op
>> +    return translate(UO->getSubExpr(), Ctx);
>> +  }
>> +
>>    // We treat these as no-ops
>> -  case UO_AddrOf:
>>    case UO_Deref:
>>    case UO_Plus:
>>      return translate(UO->getSubExpr(), Ctx);
>> @@ -360,7 +547,9 @@ til::SExpr *SExprBuilder::translateCastE
>>          return E0;
>>      }
>>      til::SExpr *E0 = translate(CE->getSubExpr(), Ctx);
>> -    return new (Arena) til::Load(E0);
>> +    return E0;
>> +    // FIXME!! -- get Load working properly
>> +    // return new (Arena) til::Load(E0);
>>    }
>>    case CK_NoOp:
>>    case CK_DerivedToBase:
>> @@ -373,6 +562,8 @@ til::SExpr *SExprBuilder::translateCastE
>>    default: {
>>      // FIXME: handle different kinds of casts.
>>      til::SExpr *E0 = translate(CE->getSubExpr(), Ctx);
>> +    if (CapabilityExprMode)
>> +      return E0;
>>      return new (Arena) til::Cast(til::CAST_none, E0);
>>    }
>>    }
>> @@ -389,15 +580,12 @@ SExprBuilder::translateArraySubscriptExp
>>
>>
>>  til::SExpr *
>> -SExprBuilder::translateConditionalOperator(const ConditionalOperator *C,
>> -                                           CallingContext *Ctx) {
>> -  return new (Arena) til::Undefined(C);
>> -}
>> -
>> -
>> -til::SExpr *SExprBuilder::translateBinaryConditionalOperator(
>> -    const BinaryConditionalOperator *C, CallingContext *Ctx) {
>> -  return new (Arena) til::Undefined(C);
>> +SExprBuilder::translateAbstractConditionalOperator(
>> +    const AbstractConditionalOperator *CO, CallingContext *Ctx) {
>> +  auto *C = translate(CO->getCond(), Ctx);
>> +  auto *T = translate(CO->getTrueExpr(), Ctx);
>> +  auto *E = translate(CO->getFalseExpr(), Ctx);
>> +  return new (Arena) til::IfThenElse(C, T, E);
>>  }
>>
>>
>> @@ -430,9 +618,7 @@ SExprBuilder::translateDeclStmt(const De
>>  // If E is trivial returns E.
>>  til::SExpr *SExprBuilder::addStatement(til::SExpr* E, const Stmt *S,
>>                                         const ValueDecl *VD) {
>> -  if (!E)
>> -    return nullptr;
>> -  if (til::ThreadSafetyTIL::isTrivial(E))
>> +  if (!E || !CurrentBB || til::ThreadSafetyTIL::isTrivial(E))
>>      return E;
>>
>>    til::Variable *V = new (Arena) til::Variable(E, VD);
>> @@ -631,7 +817,6 @@ void SExprBuilder::enterCFG(CFG *Cfg, co
>>      BB->reserveInstructions(B->size());
>>      BlockMap[B->getBlockID()] = BB;
>>    }
>> -  CallCtx.reset(new SExprBuilder::CallingContext(D));
>>
>>    CurrentBB = lookupBlock(&Cfg->getEntry());
>>    auto Parms = isa<ObjCMethodDecl>(D) ?
>> cast<ObjCMethodDecl>(D)->parameters()
>> @@ -697,7 +882,7 @@ void SExprBuilder::enterCFGBlockBody(con
>>
>>
>>  void SExprBuilder::handleStatement(const Stmt *S) {
>> -  til::SExpr *E = translate(S, CallCtx.get());
>> +  til::SExpr *E = translate(S, nullptr);
>>    addStatement(E, S);
>>  }
>>
>> @@ -730,7 +915,7 @@ void SExprBuilder::exitCFGBlockBody(cons
>>      CurrentBB->setTerminator(Tm);
>>    }
>>    else if (N == 2) {
>> -    til::SExpr *C = translate(B->getTerminatorCondition(true),
>> CallCtx.get());
>> +    til::SExpr *C = translate(B->getTerminatorCondition(true), nullptr);
>>      til::BasicBlock *BB1 = *It ? lookupBlock(*It) : nullptr;
>>      ++It;
>>      til::BasicBlock *BB2 = *It ? lookupBlock(*It) : nullptr;
>> @@ -775,18 +960,15 @@ void SExprBuilder::exitCFG(const CFGBloc
>>  }
>>
>>
>> -
>> -class TILPrinter : public til::PrettyPrinter<TILPrinter,
>> llvm::raw_ostream> {};
>> -
>> -
>> +/*
>>  void printSCFG(CFGWalker &Walker) {
>>    llvm::BumpPtrAllocator Bpa;
>>    til::MemRegionRef Arena(&Bpa);
>> -  SExprBuilder builder(Arena);
>> -  til::SCFG *Cfg = builder.buildCFG(Walker);
>> -  TILPrinter::print(Cfg, llvm::errs());
>> +  SExprBuilder SxBuilder(Arena);
>> +  til::SCFG *Scfg = SxBuilder.buildCFG(Walker);
>> +  TILPrinter::print(Scfg, llvm::errs());
>>  }
>> -
>> +*/
>>
>>
>>  } // end namespace threadSafety
>>
>> Modified: cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp (original)
>> +++ cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp Mon Jul 28 10:57:27 2014
>> @@ -88,10 +88,42 @@ void SCFG::renumberVars() {
>>
>>
>>
>> +// If E is a variable, then trace back through any aliases or redundant
>> +// Phi nodes to find the canonical definition.
>> +const SExpr *getCanonicalVal(const SExpr *E) {
>> +  while (auto *V = dyn_cast<Variable>(E)) {
>> +    const SExpr *D;
>> +    do {
>> +      if (V->kind() != Variable::VK_Let)
>> +        return V;
>> +      D = V->definition();
>> +      auto *V2 = dyn_cast<Variable>(D);
>> +      if (V2)
>> +        V = V2;
>> +      else
>> +        break;
>> +    } while (true);
>> +
>> +    if (ThreadSafetyTIL::isTrivial(D))
>> +      return D;
>> +
>> +    if (const Phi *Ph = dyn_cast<Phi>(D)) {
>> +      if (Ph->status() == Phi::PH_SingleVal) {
>> +        E = Ph->values()[0];
>> +        continue;
>> +      }
>> +    }
>> +    return V;
>> +  }
>> +  return E;
>> +}
>> +
>> +
>>
>>  // If E is a variable, then trace back through any aliases or redundant
>>  // Phi nodes to find the canonical definition.
>> -SExpr *getCanonicalVal(SExpr *E) {
>> +// The non-const version will simplify incomplete Phi nodes.
>> +SExpr *simplifyToCanonicalVal(SExpr *E) {
>>    while (auto *V = dyn_cast<Variable>(E)) {
>>      SExpr *D;
>>      do {
>> @@ -123,6 +155,7 @@ SExpr *getCanonicalVal(SExpr *E) {
>>  }
>>
>>
>> +
>>  // Trace the arguments of an incomplete Phi node to see if they have the
>> same
>>  // canonical definition.  If so, mark the Phi node as redundant.
>>  // getCanonicalVal() will recursively call simplifyIncompletePhi().
>> @@ -132,9 +165,9 @@ void simplifyIncompleteArg(Variable *V,
>>    // eliminate infinite recursion -- assume that this node is not
>> redundant.
>>    Ph->setStatus(Phi::PH_MultiVal);
>>
>> -  SExpr *E0 = getCanonicalVal(Ph->values()[0]);
>> +  SExpr *E0 = simplifyToCanonicalVal(Ph->values()[0]);
>>    for (unsigned i=1, n=Ph->values().size(); i<n; ++i) {
>> -    SExpr *Ei = getCanonicalVal(Ph->values()[i]);
>> +    SExpr *Ei = simplifyToCanonicalVal(Ph->values()[i]);
>>      if (Ei == V)
>>        continue;  // Recursive reference to itself.  Don't count.
>>      if (Ei != E0) {
>>
>> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
>> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Mon Jul 28 10:57:27 2014
>> @@ -1444,9 +1444,9 @@ struct SortDiagBySourceLocation {
>>  // -Wthread-safety
>>
>> //===----------------------------------------------------------------------===//
>>  namespace clang {
>> -namespace thread_safety {
>> -namespace {
>> -class ThreadSafetyReporter : public
>> clang::thread_safety::ThreadSafetyHandler {
>> +namespace threadSafety {
>> +
>> +class ThreadSafetyReporter : public
>> clang::threadSafety::ThreadSafetyHandler {
>>    Sema &S;
>>    DiagList Warnings;
>>    SourceLocation FunLocation, FunEndLocation;
>> @@ -1608,7 +1608,7 @@ class ThreadSafetyReporter : public clan
>>      Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
>>    }
>>  };
>> -}
>> +
>>  }
>>  }
>>
>> @@ -1896,11 +1896,11 @@ AnalysisBasedWarnings::IssueWarnings(sem
>>    if (P.enableThreadSafetyAnalysis) {
>>      SourceLocation FL = AC.getDecl()->getLocation();
>>      SourceLocation FEL = AC.getDecl()->getLocEnd();
>> -    thread_safety::ThreadSafetyReporter Reporter(S, FL, FEL);
>> +    threadSafety::ThreadSafetyReporter Reporter(S, FL, FEL);
>>      if (!Diags.isIgnored(diag::warn_thread_safety_beta,
>> D->getLocStart()))
>>        Reporter.setIssueBetaWarnings(true);
>>
>> -    thread_safety::runThreadSafetyAnalysis(AC, Reporter);
>> +    threadSafety::runThreadSafetyAnalysis(AC, Reporter);
>>      Reporter.emitDiagnostics();
>>    }
>>
>>
>> Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=214089&r1=214088&r2=214089&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Mon Jul 28
>> 10:57:27 2014
>> @@ -95,6 +95,13 @@ public:
>>  };
>>
>>
>> +template <class K, class T>
>> +class MyMap {
>> +public:
>> +  T& operator[](const K& k);
>> +};
>> +
>> +
>>
>>  Mutex sls_mu;
>>
>> @@ -2280,6 +2287,15 @@ void test() {
>>    (a > 0 ? fooArray[1] : fooArray[b]).mu_.Lock();
>>    (a > 0 ? fooArray[1] : fooArray[b]).a = 0;
>>    (a > 0 ? fooArray[1] : fooArray[b]).mu_.Unlock();
>> +}
>> +
>> +
>> +void test2() {
>> +  Foo *fooArray;
>> +  Bar bar;
>> +  int a;
>> +  int b;
>> +  int c;
>>
>>    bar.getFoo().mu_.Lock();
>>    bar.getFooey().a = 0; // \
>> @@ -2295,20 +2311,20 @@ void test() {
>>
>>    bar.getFoo3(a, b).mu_.Lock();
>>    bar.getFoo3(a, c).a = 0;  // \
>> -    // expected-warning {{writing variable 'a' requires holding mutex
>> 'bar.getFoo3(a,c).mu_' exclusively}} \
>> -    // expected-note {{'bar.getFoo3(a,b).mu_'}}
>> +    // expected-warning {{writing variable 'a' requires holding mutex
>> 'bar.getFoo3(a, c).mu_' exclusively}} \
>> +    // expected-note {{found near match 'bar.getFoo3(a, b).mu_'}}
>>    bar.getFoo3(a, b).mu_.Unlock();
>>
>>    getBarFoo(bar, a).mu_.Lock();
>>    getBarFoo(bar, b).a = 0;  // \
>> -    // expected-warning {{writing variable 'a' requires holding mutex
>> 'getBarFoo(bar,b).mu_' exclusively}} \
>> -    // expected-note {{'getBarFoo(bar,a).mu_'}}
>> +    // expected-warning {{writing variable 'a' requires holding mutex
>> 'getBarFoo(bar, b).mu_' exclusively}} \
>> +    // expected-note {{found near match 'getBarFoo(bar, a).mu_'}}
>>    getBarFoo(bar, a).mu_.Unlock();
>>
>>    (a > 0 ? fooArray[1] : fooArray[b]).mu_.Lock();
>>    (a > 0 ? fooArray[b] : fooArray[c]).a = 0; // \
>> -    // expected-warning {{writing variable 'a' requires holding mutex
>> '((a#_)#_#fooArray[b]).mu_' exclusively}} \
>> -    // expected-note {{'((a#_)#_#fooArray[_]).mu_'}}
>> +    // expected-warning {{writing variable 'a' requires holding mutex
>> '((0 < a) ? fooArray[b] : fooArray[c]).mu_' exclusively}} \
>> +    // expected-note {{found near match '((0 < a) ? fooArray[1] :
>> fooArray[b]).mu_'}}
>>    (a > 0 ? fooArray[1] : fooArray[b]).mu_.Unlock();
>>  }
>>
>> @@ -4378,3 +4394,126 @@ class Foo {
>>  };
>>
>>  }  // end namespace ThreadAttributesOnLambdas
>> +
>> +
>> +
>> +namespace AttributeExpressionCornerCases {
>> +
>> +class Foo {
>> +  int a GUARDED_BY(getMu());
>> +
>> +  Mutex* getMu()   LOCK_RETURNED("");
>> +  Mutex* getUniv() LOCK_RETURNED("*");
>> +
>> +  void test1() {
>> +    a = 0;
>> +  }
>> +
>> +  void test2() EXCLUSIVE_LOCKS_REQUIRED(getUniv()) {
>> +    a = 0;
>> +  }
>> +
>> +  void foo(Mutex* mu) EXCLUSIVE_LOCKS_REQUIRED(mu);
>> +
>> +  void test3() {
>> +    foo(nullptr);
>> +  }
>> +};
>> +
>> +
>> +class MapTest {
>> +  struct MuCell { Mutex* mu; };
>> +
>> +  MyMap<MyString, Mutex*> map;
>> +  MyMap<MyString, MuCell> mapCell;
>> +
>> +  int a GUARDED_BY(map["foo"]);
>> +  int b GUARDED_BY(mapCell["foo"].mu);
>> +
>> +  void test() {
>> +    map["foo"]->Lock();
>> +    a = 0;
>> +    map["foo"]->Unlock();
>> +  }
>> +
>> +  void test2() {
>> +    mapCell["foo"].mu->Lock();
>> +    b = 0;
>> +    mapCell["foo"].mu->Unlock();
>> +  }
>> +};
>> +
>> +
>> +class PreciseSmartPtr {
>> +  SmartPtr<Mutex> mu;
>> +  int val GUARDED_BY(mu);
>> +
>> +  static bool compare(PreciseSmartPtr& a, PreciseSmartPtr &b) {
>> +    a.mu->Lock();
>> +    bool result = (a.val == b.val);   // expected-warning {{reading
>> variable 'val' requires holding mutex 'b.mu'}} \
>> +                                      // expected-note {{found near match
>> 'a.mu'}}
>> +    a.mu->Unlock();
>> +    return result;
>> +  }
>> +};
>> +
>> +
>> +class SmartRedeclare {
>> +  SmartPtr<Mutex> mu;
>> +  int val GUARDED_BY(mu);
>> +
>> +  void test()  EXCLUSIVE_LOCKS_REQUIRED(mu);
>> +  void test2() EXCLUSIVE_LOCKS_REQUIRED(mu.get());
>> +  void test3() EXCLUSIVE_LOCKS_REQUIRED(mu.get());
>> +};
>> +
>> +
>> +void SmartRedeclare::test() EXCLUSIVE_LOCKS_REQUIRED(mu.get()) {
>> +  val = 0;
>> +}
>> +
>> +void SmartRedeclare::test2() EXCLUSIVE_LOCKS_REQUIRED(mu) {
>> +  val = 0;
>> +}
>> +
>> +void SmartRedeclare::test3() {
>> +  val = 0;
>> +}
>> +
>> +
>> +namespace CustomMutex {
>> +
>> +
>> +class LOCKABLE BaseMutex { };
>> +class DerivedMutex : public BaseMutex { };
>> +
>> +void customLock(const BaseMutex *m)   EXCLUSIVE_LOCK_FUNCTION(m);
>> +void customUnlock(const BaseMutex *m) UNLOCK_FUNCTION(m);
>> +
>> +static struct DerivedMutex custMu;
>> +
>> +static void doSomethingRequiringLock() EXCLUSIVE_LOCKS_REQUIRED(custMu) {
>> }
>> +
>> +void customTest() {
>> +  customLock(reinterpret_cast<BaseMutex*>(&custMu));  // ignore casts
>> +  doSomethingRequiringLock();
>> +  customUnlock(reinterpret_cast<BaseMutex*>(&custMu));
>> +}
>> +
>> +} // end namespace CustomMutex
>> +
>> +} // end AttributeExpressionCornerCases
>> +
>> +
>> +namespace ScopedLockReturnedInvalid {
>> +
>> +class Opaque;
>> +
>> +Mutex* getMutex(Opaque* o) LOCK_RETURNED("");
>> +
>> +void test(Opaque* o) {
>> +  MutexLock lock(getMutex(o));
>> +}
>> +
>> +}  // end namespace ScopedLockReturnedInvalid
>> +
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>



-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315



More information about the cfe-commits mailing list