Merge remote-tracking branch 'origin/main' into no_extra_atoms

Author: kyleheadley

clang/include/clang/3C/ConstraintVariables.h

@@ -325,6 +325,7 @@ class PointerVariableConstraint : public ConstraintVariable {
   // String representing declared bounds expression.
   std::string BoundsAnnotationStr;
 
+  // TODO can we move this to an optional instead of the -1?
   // Does this variable represent a generic type? Which one (or -1 for none)?
   // Generic types can be used with fewer restrictions, so this field is used
   // stop assignments with generic variables from forcing constraint variables

clang/include/clang/3C/TypeVariableAnalysis.h

@@ -33,7 +33,9 @@ class TypeVariableEntry {
   }
 
   bool getIsConsistent() const;
+  // Note: undefined behaviour if `getIsConsistent` is false
   QualType getType();
+  // Note: undefined behaviour if `getIsConsistent` is false
   std::set<ConstraintVariable *> &getConstraintVariables();
   ConstraintVariable *getTypeParamConsVar();
 
@@ -53,6 +55,7 @@ class TypeVariableEntry {
 
   // Collection of constraint variables generated for all uses of the type
   // variable. Also should not be used when IsConsistent is false.
+  // TODO: accessor methods don't enforce this?
   std::set<ConstraintVariable *> ArgConsVars;
 
   // A single constraint variable for solving the checked type of the type
@@ -65,6 +68,7 @@ class TypeVariableEntry {
 // typed parameter. The values in the map are another maps from type variable
 // index in the called function's parameter list to the type the type variable
 // becomes (or null if it is not used consistently).
+// TODO: use a better map implementation?
 typedef std::map<CallExpr *, std::map<unsigned int, TypeVariableEntry>>
     TypeVariableMapT;
 
@@ -95,8 +99,7 @@ class TypeVarVisitor : public RecursiveASTVisitor<TypeVarVisitor>,
   ConstraintResolver CR;
   TypeVariableMapT TVMap;
 
-  void insertBinding(CallExpr *CE, const int TyIdx, QualType Ty, CVarSet &CVs,
-                     bool ForceInconsistent = false);
+  void insertBinding(CallExpr *CE, const int TyIdx, QualType Ty, CVarSet &CVs);
 };
 
 bool typeArgsProvided(CallExpr *Call);

clang/lib/3C/TypeVariableAnalysis.cpp

@@ -72,7 +72,7 @@ bool TypeVarVisitor::VisitCastExpr(CastExpr *CE) {
   if (CHKCBindTemporaryExpr *TempE = dyn_cast<CHKCBindTemporaryExpr>(SubExpr))
     SubExpr = TempE->getSubExpr();
 
-  if (auto *Call = dyn_cast<CallExpr>(SubExpr))
+  if (auto *Call = dyn_cast<CallExpr>(SubExpr)) {
     if (auto *FD = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl())) {
       FunctionDecl *FDef = getDefinition(FD);
       if (FDef == nullptr)
@@ -87,6 +87,7 @@ bool TypeVarVisitor::VisitCastExpr(CastExpr *CE) {
         }
       }
     }
+  }
   return true;
 }
 
@@ -96,11 +97,6 @@ bool TypeVarVisitor::VisitCallExpr(CallExpr *CE) {
     if (FDef == nullptr)
       FDef = FD;
     if (auto *FVCon = Info.getFuncConstraint(FDef, Context)) {
-      // if we need to rewrite it but can't (macro, etc), it isn't safe
-      bool ForcedInconsistent =
-          !typeArgsProvided(CE) &&
-          (!Rewriter::isRewritable(CE->getExprLoc()) ||
-           !canWrite(PersistentSourceLoc::mkPSL(CE, *Context).getFileName()));
       // Visit each function argument, and if it use a type variable, insert it
       // into the type variable binding map.
       unsigned int I = 0;
@@ -113,7 +109,7 @@ bool TypeVarVisitor::VisitCallExpr(CallExpr *CE) {
           Expr *Uncast = A->IgnoreImpCasts();
           std::set<ConstraintVariable *> CVs =
               CR.getExprConstraintVarsSet(Uncast);
-          insertBinding(CE, TyIdx, Uncast->getType(), CVs, ForcedInconsistent);
+          insertBinding(CE, TyIdx, Uncast->getType(), CVs);
         }
         ++I;
       }
@@ -165,8 +161,13 @@ bool TypeVarVisitor::VisitCallExpr(CallExpr *CE) {
 // the exact type variable is identified by the call expression where it is
 // used and the index of the type variable type in the function declaration.
 void TypeVarVisitor::insertBinding(CallExpr *CE, const int TyIdx,
-                                   clang::QualType Ty, CVarSet &CVs,
-                                   bool ForceInconsistent) {
+                                   clang::QualType Ty, CVarSet &CVs) {
+  // if we need to rewrite it but can't (macro, etc), it isn't safe
+  bool ForceInconsistent =
+      !typeArgsProvided(CE) &&
+      (!Rewriter::isRewritable(CE->getExprLoc()) ||
+       !canWrite(PersistentSourceLoc::mkPSL(CE, *Context).getFileName()));
+
   assert(TyIdx >= 0 &&
          "Creating a type variable binding without a type variable.");
   auto &CallTypeVarMap = TVMap[CE];

clang/test/3C/canwrite_constraints.h

@@ -71,11 +71,17 @@ void unwritable_func_with_itype_and_bounds(int *p
 // expected-warning@+1 {{Declaration in non-writable file}}
 _Itype_for_any(T) void my_generic_function(void *p : itype(_Ptr<T>));
 
+// expected-warning@+1 {{Declaration in non-writable file}}
+_Itype_for_any(T) void *my_generic_return(void) : itype(_Ptr<T>);
+
 void unwritable_type_argument() {
   int i;
+  int *b; // expected-warning {{Declaration in non-writable file}}
   // This warning relates to the atom representing the temporary pointer of
   // `&i`. https://github.com/correctcomputation/checkedc-clang/issues/618 would
   // make 3C smarter to avoid the need to constrain the temporary pointer.
   // expected-warning@+1 {{Expression in non-writable file}}
   my_generic_function(&i);
+  
+  b = my_generic_return(); // expected-warning {{Expression in non-writable file}}
 }