Refactoring to remove singleton ConstraintVariable sets

clang/include/clang/CConv/CheckedRegions.h

@@ -83,8 +83,8 @@ class CheckedRegionFinder : public clang::RecursiveASTVisitor<CheckedRegionFinde
     bool containsUncheckedPtr(clang::QualType Qt);
     bool containsUncheckedPtrAcc(clang::QualType Qt, std::set<std::string> &Seen);
     bool isUncheckedStruct(clang::QualType Qt, std::set<std::string> &Seen);
-    bool isWild(std::set<ConstraintVariable*>&);
-    bool isWild(std::set<FVConstraint*>*);
+    bool isWild(const std::set<ConstraintVariable*>&);
+    bool isWild(const std::set<FVConstraint*>*);
 
     clang::ASTContext* Context;
     clang::Rewriter& Writer;

clang/include/clang/CConv/ProgramInfo.h

@@ -45,18 +45,18 @@ class ProgramInfo : public ProgramVariableAdder {
   typedef std::map<PersistentSourceLoc, CallTypeParamBindingsT>
       TypeParamBindingsT;
 
-  typedef std::map<std::string, std::map<std::string, std::set<FVConstraint *>>>
-      StaticFunctionMapType;
 
-  typedef std::map<std::string, std::set<FVConstraint *>>
-      ExternalFunctionMapType;
+  typedef std::map<std::string, FVConstraint *> ExternalFunctionMapType;
+  typedef std::map<std::string, ExternalFunctionMapType> StaticFunctionMapType;
 
   ProgramInfo();
   void print(llvm::raw_ostream &O) const;
   void dump() const { print(llvm::errs()); }
   void dump_json(llvm::raw_ostream &O) const;
-  void dump_stats(std::set<std::string> &F) { print_stats(F, llvm::errs()); }
-  void print_stats(std::set<std::string> &F, llvm::raw_ostream &O,
+  void dump_stats(const std::set<std::string> &F) {
+    print_stats(F, llvm::errs());
+  }
+  void print_stats(const std::set<std::string> &F, llvm::raw_ostream &O,
                    bool OnlySummary = false, bool JsonFormat = false);
 
   // Populate Variables, VarDeclToStatement, RVariables, and DepthMap with
@@ -69,16 +69,15 @@ class ProgramInfo : public ProgramVariableAdder {
   // should all be empty. 
   void exitCompilationUnit();
 
-  CVarSet
-      &getPersistentConstraintVars(Expr *E, ASTContext *AstContext);
+  CVarSet &getPersistentConstraintVars(Expr *E, ASTContext *AstContext);
   // Get constraint variable for the provided Decl
-  CVarSet getVariable(clang::Decl *D,
-                                             clang::ASTContext *C);
+  CVarSet getVariable(clang::Decl *D, clang::ASTContext *C);
 
   // Retrieve a function's constraints by decl, or by name; nullptr if not found
-  std::set<FVConstraint *> *getFuncConstraints(FunctionDecl *D, ASTContext *C);
-  std::set<FVConstraint *> *getExtFuncDefnConstraintSet(std::string FuncName);
-  std::set<FVConstraint *> *getStaticFuncConstraintSet(std::string FuncName, std::string FileName);
+  FVConstraint *getFuncConstraints (FunctionDecl *D, ASTContext *C) const;
+  FVConstraint *getExtFuncDefnConstraintSet (std::string FuncName) const;
+  FVConstraint *getStaticFuncConstraintSet(std::string FuncName,
+                                           std::string FileName) const;
 
   // Check if the given function is an extern function.
   bool isAnExternFunction(const std::string &FName);
@@ -93,26 +92,27 @@ class ProgramInfo : public ProgramVariableAdder {
   AVarBoundsInfo &getABoundsInfo() { return ArrBInfo; }
 
   // Parameter map is used for cast insertion, post-rewriting
-  void merge_MF(ParameterMap &MF);
+  void merge_MF(const ParameterMap &MF);
   ParameterMap &getMF();
 
   ConstraintsInfo &getInterimConstraintState() {
     return CState;
   }
-  bool computeInterimConstraintState(std::set<std::string> &FilePaths);
+  bool computeInterimConstraintState(const std::set<std::string> &FilePaths);
 
-  ExternalFunctionMapType &getExternFuncDefFVMap() {
+  const ExternalFunctionMapType &getExternFuncDefFVMap() const {
     return ExternalFunctionFVCons;
   }
 
-  StaticFunctionMapType &getStaticFuncDefFVMap() {
+  const StaticFunctionMapType &getStaticFuncDefFVMap() const {
     return StaticFunctionFVCons;
   }
 
   void setTypeParamBinding(CallExpr *CE, unsigned int TypeVarIdx,
                            ConstraintVariable *CV, ASTContext *C);
-  bool hasTypeParamBindings(CallExpr *CE, ASTContext *C);
-  CallTypeParamBindingsT &getTypeParamBindings(CallExpr *CE, ASTContext *C);
+  bool hasTypeParamBindings(CallExpr *CE, ASTContext *C) const;
+  const CallTypeParamBindingsT &getTypeParamBindings(CallExpr *CE,
+                                                     ASTContext *C) const;
 
   void constrainWildIfMacro(CVarSet &S, SourceLocation Location);
 
@@ -156,14 +156,14 @@ class ProgramInfo : public ProgramVariableAdder {
   // Returns true if successful else false.
   bool insertIntoExternalFunctionMap(ExternalFunctionMapType &Map,
                                      const std::string &FuncName,
-                                     std::set<FVConstraint *> &ToIns);
+                                     FVConstraint *ToIns);
 
   // Inserts the given FVConstraint* set into the provided static map.
   // Returns true if successful else false.
   bool insertIntoStaticFunctionMap(StaticFunctionMapType &Map,
                                    const std::string &FuncName,
                                    const std::string &FileName,
-                                   std::set<FVConstraint *> &ToIns);
+                                   FVConstraint *ToIns);
 
 
   // Special-case handling for decl introductions. For the moment this covers:
@@ -173,14 +173,12 @@ class ProgramInfo : public ProgramVariableAdder {
 
   // Inserts the given FVConstraint* set into the global map, depending
   // on whether static or not; returns true on success
-  bool
-  insertNewFVConstraints(FunctionDecl *FD, std::set<FVConstraint *> &FVcons,
-                         ASTContext *C);
-
-  // Retrieves a FVConstraint* based on the decl (which could be static,
-  //   or global)
-  std::set<FVConstraint *> *getFuncFVConstraints(FunctionDecl *FD,
-                                                 ASTContext *C);
+  bool insertNewFVConstraints(FunctionDecl *FD, FVConstraint *FVcons,
+                              ASTContext *C);
+
+  // Retrieves a FVConstraint* from a Decl (which could be static, or global)
+  FVConstraint *getFuncFVConstraints(FunctionDecl *FD, ASTContext *C);
+
   // For each pointer type in the declaration of D, add a variable to the
   // constraint system for that pointer type.
   void addVariable(clang::DeclaratorDecl *D, clang::ASTContext *astContext);

clang/include/clang/CConv/Utils.h

@@ -61,7 +61,7 @@ class BiMap {
 extern std::set<std::string> FilePaths;
 
 template <typename T>
-T getOnly(std::set<T> &singletonSet) {
+T getOnly(const std::set<T> &singletonSet) {
   assert(singletonSet.size() == 1);
   return (*singletonSet.begin());
 }

clang/lib/CConv/AVarBoundsInfo.cpp

@@ -333,7 +333,7 @@ void AVarBoundsInfo::insertProgramVar(BoundsKey NK, ProgramVar *PV) {
   PVarInfo[NK] = PV;
 }
 
-bool hasArray(CVarSet &CSet, Constraints &CS) {
+bool hasArray(const CVarSet &CSet, Constraints &CS) {
   auto &E = CS.getVariables();
   for (auto *CK : CSet) {
     if (PVConstraint *PV = dyn_cast<PVConstraint>(CK)) {
@@ -346,7 +346,7 @@ bool hasArray(CVarSet &CSet, Constraints &CS) {
   return false;
 }
 
-bool isInSrcArray(CVarSet &CSet, Constraints &CS) {
+bool isInSrcArray(const CVarSet &CSet, Constraints &CS) {
   auto &E = CS.getVariables();
   for (auto *CK : CSet) {
     if (PVConstraint *PV = dyn_cast<PVConstraint>(CK)) {
@@ -674,9 +674,9 @@ bool AVarBoundsInfo::performFlowAnalysis(ProgramInfo *PI) {
       unsigned ParmNum = std::get<3>(ParmTup);
       FVConstraint *FV = nullptr;
       if (IsStatic || !PI->getExtFuncDefnConstraintSet(FuncName)) {
-        FV = getOnly(*(PI->getStaticFuncConstraintSet(FuncName, FileName)));
+        FV = PI->getStaticFuncConstraintSet(FuncName, FileName);
       } else {
-        FV = getOnly(*(PI->getExtFuncDefnConstraintSet(FuncName)));
+        FV = PI->getExtFuncDefnConstraintSet(FuncName);
       }
 
       if (hasArray(FV->getParamVar(ParmNum), CS)) {

clang/lib/CConv/CastPlacement.cpp

@@ -20,58 +20,52 @@ bool CastPlacementVisitor::VisitCallExpr(CallExpr *CE) {
   auto *FD = dyn_cast_or_null<FunctionDecl>(CE->getCalleeDecl());
   if (FD && Rewriter::isRewritable(CE->getExprLoc())) {
     // Get the constraint variable for the function.
-    std::set<FVConstraint *> *V = Info.getFuncConstraints(FD, Context);
+    FVConstraint *FV = Info.getFuncConstraints(FD, Context);
     // Function has no definition i.e., external function.
-    assert("Function has no definition" && V != nullptr);
+    assert("Function has no definition" && FV != nullptr);
 
     // Did we see this function in another file?
     auto Fname = FD->getNameAsString();
-    if (!V->empty() && !ConstraintResolver::canFunctionBeSkipped(Fname)) {
-      // Get the FV constraint for the Callee.
-      // TODO: This seems to assume that V is a singleton set. Validate this
-      //       assumption and use getOnly.
-      if (FVConstraint *FV = *(V->begin())) {
-        // Now we need to check the type of the arguments and corresponding
-        // parameters to see if any explicit casting is needed.
-        ProgramInfo::CallTypeParamBindingsT TypeVars;
-        if (Info.hasTypeParamBindings(CE, Context))
-          TypeVars = Info.getTypeParamBindings(CE, Context);
-        auto PInfo = Info.getMF()[Fname];
-        unsigned PIdx = 0;
-        for (const auto &A : CE->arguments()) {
-          if (PIdx < FD->getNumParams()) {
+    if (!ConstraintResolver::canFunctionBeSkipped(Fname)) {
+      // Now we need to check the type of the arguments and corresponding
+      // parameters to see if any explicit casting is needed.
+      ProgramInfo::CallTypeParamBindingsT TypeVars;
+      if (Info.hasTypeParamBindings(CE, Context))
+        TypeVars = Info.getTypeParamBindings(CE, Context);
+      auto PInfo = Info.getMF()[Fname];
+      unsigned PIdx = 0;
+      for (const auto &A : CE->arguments()) {
+        if (PIdx < FD->getNumParams()) {
+          // Avoid adding incorrect casts to generic function arguments by
+          // removing implicit casts when on arguments with a consistently
+          // used generic type.
+          Expr *ArgExpr = A;
+          const TypeVariableType
+              *TyVar = getTypeVariableType(FD->getParamDecl(PIdx));
+          if (TyVar && TypeVars.find(TyVar->GetIndex()) != TypeVars.end()
+              && TypeVars[TyVar->GetIndex()] != nullptr)
+            ArgExpr = ArgExpr->IgnoreImpCasts();
 
-            // Avoid adding incorrect casts to generic function arguments by
-            // removing implicit casts when on arguments with a consistently
-            // used generic type.
-            Expr *ArgExpr = A;
-            const TypeVariableType
-                *TyVar = getTypeVariableType(FD->getParamDecl(PIdx));
-            if (TyVar && TypeVars.find(TyVar->GetIndex()) != TypeVars.end()
-                && TypeVars[TyVar->GetIndex()] != nullptr)
-              ArgExpr = ArgExpr->IgnoreImpCasts();
-
-            CVarSet ArgumentConstraints = CR.getExprConstraintVars(ArgExpr);
-            CVarSet &ParameterConstraints = FV->getParamVar(PIdx);
-            for (auto *ArgumentC : ArgumentConstraints) {
-              bool CastInserted = false;
-              for (auto *ParameterC : ParameterConstraints) {
-                auto Dinfo = PIdx < PInfo.size() ? PInfo[PIdx] : CHECKED;
-                if (needCasting(ArgumentC, ParameterC, Dinfo)) {
-                  // We expect the cast string to end with "(".
-                  std::string CastString =
-                      getCastString(ArgumentC, ParameterC, Dinfo);
-                  surroundByCast(CastString, A);
-                  CastInserted = true;
-                  break;
-                }
+          CVarSet ArgumentConstraints = CR.getExprConstraintVars(ArgExpr);
+          const CVarSet &ParameterConstraints = FV->getParamVar(PIdx);
+          for (auto *ArgumentC : ArgumentConstraints) {
+            bool CastInserted = false;
+            for (auto *ParameterC : ParameterConstraints) {
+              auto Dinfo = PIdx < PInfo.size() ? PInfo[PIdx] : CHECKED;
+              if (needCasting(ArgumentC, ParameterC, Dinfo)) {
+                // We expect the cast string to end with "(".
+                std::string CastString =
+                    getCastString(ArgumentC, ParameterC, Dinfo);
+                surroundByCast(CastString, A);
+                CastInserted = true;
+                break;
               }
-              // If we have already inserted a cast, then break.
-              if (CastInserted) break;
             }
+            // If we have already inserted a cast, then break.
+            if (CastInserted) break;
           }
-          PIdx++;
         }
+        PIdx++;
       }
     }
   }

clang/lib/CConv/CheckedRegions.cpp

@@ -235,8 +235,8 @@ bool CheckedRegionFinder::VisitDeclRefExpr(DeclRefExpr* DR) {
   bool IW = isWild(CVSet ) || containsUncheckedPtr(T);
 
   if (auto FD = dyn_cast<FunctionDecl>(D)) {
-    auto FV = Info.getFuncConstraints(FD, Context);
-    IW |= isWild(FV);
+    auto *FV = Info.getFuncConstraints(FD, Context);
+    IW |= FV->hasWild(Info.getConstraints().getVariables());
     for (const auto& param: FD->parameters()) {
       auto CVSet = Info.getVariable(param, Context);
       IW |= isWild(CVSet);
@@ -298,15 +298,15 @@ bool CheckedRegionFinder::isInStatementPosition(CallExpr *C) {
   }
 }
 
-bool CheckedRegionFinder::isWild(std::set<ConstraintVariable*> &S) { 
+bool CheckedRegionFinder::isWild(const std::set<ConstraintVariable*> &S) {
   for (auto Cv : S) 
     if (Cv->hasWild(Info.getConstraints().getVariables()))
       return true;
 
   return false;
 }
 
-bool CheckedRegionFinder::isWild(std::set<FVConstraint*> *S) {
+bool CheckedRegionFinder::isWild(const std::set<FVConstraint*> *S) {
   for (auto Fv : *S)
     if (Fv->hasWild(Info.getConstraints().getVariables()))
       return true;