Soften error#414

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

@@ -138,7 +138,8 @@ class ConstraintVariable {
   virtual void equateArgumentConstraints(ProgramInfo &I) = 0;
 
   // Update this CV with information from duplicate declaration CVs
-  virtual void brainTransplant(ConstraintVariable *, ProgramInfo &) = 0;
+  virtual void brainTransplant(ConstraintVariable *, ProgramInfo &,
+                               std::string &ReasonFailed) = 0;
   virtual void mergeDeclaration(ConstraintVariable *, ProgramInfo &,
                                 std::string &ReasonFailed) = 0;
 
@@ -383,7 +384,8 @@ class PointerVariableConstraint : public ConstraintVariable {
 
   const CAtoms &getCvars() const { return Vars; }
 
-  void brainTransplant(ConstraintVariable *From, ProgramInfo &I) override;
+  void brainTransplant(ConstraintVariable *From, ProgramInfo &I,
+                       std::string &ReasonFailed) override;
   void mergeDeclaration(ConstraintVariable *From, ProgramInfo &I,
                         std::string &ReasonFailed) override;
 
@@ -466,7 +468,8 @@ class FVComponentVariable {
 
   void mergeDeclaration(FVComponentVariable *From, ProgramInfo &I,
                         std::string &ReasonFailed);
-  void brainTransplant(FVComponentVariable *From, ProgramInfo &I);
+  void brainTransplant(FVComponentVariable *From, ProgramInfo &I,
+                       std::string &ReasonFailed);
 
   std::string mkItypeStr(const EnvironmentMap &E) const;
   std::string mkTypeStr(const EnvironmentMap &E) const;
@@ -536,7 +539,8 @@ class FunctionVariableConstraint : public ConstraintVariable {
     return S->getKind() == FunctionVariable;
   }
 
-  void brainTransplant(ConstraintVariable *From, ProgramInfo &I) override;
+  void brainTransplant(ConstraintVariable *From, ProgramInfo &I,
+                       std::string &ReasonFailed) override;
   void mergeDeclaration(ConstraintVariable *FromCV, ProgramInfo &I,
                         std::string &ReasonFailed) override;
 

clang/include/clang/3C/ProgramInfo.h

@@ -167,20 +167,7 @@ class ProgramInfo : public ProgramVariableAdder {
   // For each call to a generic function, remember how the type parameters were
   // instantiated so they can be inserted during rewriting.
   TypeParamBindingsT TypeParamBindings;
-
-  // Insert the given FVConstraint* set into the provided Map.
-  void insertIntoExternalFunctionMap(ExternalFunctionMapType &Map,
-                                     const std::string &FuncName,
-                                     FVConstraint *NewC, FunctionDecl *FD,
-                                     ASTContext *C);
-
-  // Inserts the given FVConstraint* set into the provided static map.
-  void insertIntoStaticFunctionMap(StaticFunctionMapType &Map,
-                                   const std::string &FuncName,
-                                   const std::string &FileName,
-                                   FVConstraint *ToIns, FunctionDecl *FD,
-                                   ASTContext *C);
-
+
   // Special-case handling for decl introductions. For the moment this covers:
   //  * void-typed variables
   //  * va_list-typed variables

clang/lib/3C/ConstraintVariables.cpp

@@ -1724,16 +1724,21 @@ bool isAValidPVConstraint(const ConstraintVariable *C) {
 
 // Replace CVars and ArgumentConstraints with those in [FromCV].
 void PointerVariableConstraint::brainTransplant(ConstraintVariable *FromCV,
-                                                ProgramInfo &I) {
+                                                ProgramInfo &I,
+                                                std::string &ReasonFailed) {
   PVConstraint *From = dyn_cast<PVConstraint>(FromCV);
   assert(From != nullptr);
   CAtoms CFrom = From->getCvars();
-  assert(Vars.size() == CFrom.size());
+  if (Vars.size() != CFrom.size()) {
+    ReasonFailed = "transplanting between pointers with different depths";
+    return;
+  }
   if (From->hasBoundsKey()) {
     // If this has bounds key!? Then do brain transplant of
     // bound keys as well.
     if (hasBoundsKey())
-      I.getABoundsInfo().brainTransplant(getBoundsKey(), From->getBoundsKey());
+      I.getABoundsInfo().brainTransplant(getBoundsKey(),
+                                         From->getBoundsKey());
 
     ValidBoundsKey = From->hasBoundsKey();
     BKey = From->getBoundsKey();
@@ -1742,7 +1747,11 @@ void PointerVariableConstraint::brainTransplant(ConstraintVariable *FromCV,
   ArgumentConstraints = From->getArgumentConstraints();
   if (FV) {
     assert(From->FV);
-    FV->brainTransplant(From->FV, I);
+    FV->brainTransplant(From->FV, I, ReasonFailed);
+    if (ReasonFailed != "") {
+      ReasonFailed += " within the referenced function";
+      return;
+    }
   }
 }
 
@@ -1755,7 +1764,7 @@ void PointerVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV,
   CAtoms::iterator I = Vars.begin();
   CAtoms::iterator J = CFrom.begin();
   if (CFrom.size() != Vars.size()) {
-    ReasonFailed = "conflicting types ";
+    ReasonFailed = "transplanting between pointers with different depths";
     return;
   }
   while (I != Vars.end()) {
@@ -1783,7 +1792,7 @@ void PointerVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV,
     ++I;
     ++J;
   }
-  assert(Vars.size() == NewVatoms.size() && "Merging Failed");
+  assert(Vars.size() == NewVatoms.size() && "Merging Error");
   Vars = NewVatoms;
   SrcHasItype = SrcHasItype || From->SrcHasItype;
   if (!From->ItypeStr.empty())
@@ -1795,6 +1804,10 @@ void PointerVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV,
   if (FV) {
     assert(From->FV);
     FV->mergeDeclaration(From->FV, Info, ReasonFailed);
+    if (ReasonFailed != "") {
+      ReasonFailed += " within the referenced function";
+      return;
+    }
   }
 }
 
@@ -1816,15 +1829,26 @@ Atom *PointerVariableConstraint::getAtom(unsigned AtomIdx, Constraints &CS) {
 
 // Brain Transplant params and returns in [FromCV], recursively.
 void FunctionVariableConstraint::brainTransplant(ConstraintVariable *FromCV,
-                                                 ProgramInfo &I) {
+                                                 ProgramInfo &I,
+                                                 std::string &ReasonFailed) {
   FVConstraint *From = dyn_cast<FVConstraint>(FromCV);
   assert(From != nullptr);
   // Transplant returns.
-  ReturnVar.brainTransplant(&From->ReturnVar, I);
+  ReturnVar.brainTransplant(&From->ReturnVar, I, ReasonFailed);
+  if (ReasonFailed != "") {
+    ReasonFailed += " for return value";
+    return;
+  }
+
   // Transplant params.
   if (numParams() == From->numParams()) {
-    for (unsigned J = 0; J < From->numParams(); J++)
-      ParamVars[J].brainTransplant(&From->ParamVars[J], I);
+    for (unsigned J = 0; J < From->numParams(); J++) {
+      ParamVars[J].brainTransplant(&From->ParamVars[J], I, ReasonFailed);
+      if (ReasonFailed != "") {
+        ReasonFailed += " for parameter " + std::to_string(J);
+        return;
+      }
+    }
   } else if (numParams() != 0 && From->numParams() == 0) {
     auto &CS = I.getConstraints();
     const std::vector<ParamDeferment> &Defers = From->getDeferredParams();
@@ -1839,7 +1863,7 @@ void FunctionVariableConstraint::brainTransplant(ConstraintVariable *FromCV,
       }
     }
   } else {
-    llvm_unreachable("Brain Transplant on empty params");
+    ReasonFailed = "transplant on differing param count";
   }
 }
 
@@ -1855,7 +1879,7 @@ void FunctionVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV,
   // Transplant returns.
   ReturnVar.mergeDeclaration(&From->ReturnVar, I, ReasonFailed);
   if (ReasonFailed != "") {
-    ReasonFailed += "for return value";
+    ReasonFailed += " for return value";
     return;
   }
 
@@ -1865,7 +1889,7 @@ void FunctionVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV,
   }
   if (this->numParams() == 0) {
     // This is an untyped declaration, we need to perform a transplant
-    From->brainTransplant(this, I);
+    From->brainTransplant(this, I, ReasonFailed);
   } else {
     // Standard merge
     if (this->numParams() != From->numParams()) {
@@ -1875,7 +1899,7 @@ void FunctionVariableConstraint::mergeDeclaration(ConstraintVariable *FromCV,
     for (unsigned J = 0; J < From->numParams(); J++) {
       ParamVars[J].mergeDeclaration(&From->ParamVars[J], I, ReasonFailed);
       if (ReasonFailed != "") {
-        ReasonFailed += "for parameter " + std::to_string(J);
+        ReasonFailed += " for parameter " + std::to_string(J);
         return;
       }
     }
@@ -1908,19 +1932,30 @@ void FVComponentVariable::mergeDeclaration(FVComponentVariable *From,
     InternalConstraint->mergeDeclaration(From->InternalConstraint, I,
                                          ReasonFailed);
   }
+  if (ReasonFailed != ""){
+    ReasonFailed += " during internal merge";
+    return;
+  }
   ExternalConstraint->mergeDeclaration(From->ExternalConstraint, I,
                                        ReasonFailed);
 }
 
 void FVComponentVariable::brainTransplant(FVComponentVariable *From,
-                                          ProgramInfo &I) {
+                                          ProgramInfo &I,
+                                          std::string &ReasonFailed) {
   // As in mergeDeclaration, special handling is required if the original
   // declaration did not allocate split constraint variables.
   if (InternalConstraint == ExternalConstraint)
     InternalConstraint = From->InternalConstraint;
   else
-    InternalConstraint->brainTransplant(From->InternalConstraint, I);
-  ExternalConstraint->brainTransplant(From->ExternalConstraint, I);
+    InternalConstraint->brainTransplant(From->InternalConstraint, I,
+                                        ReasonFailed);
+  if (ReasonFailed != ""){
+    ReasonFailed += " during internal merge";
+    return;
+  }
+  ExternalConstraint->brainTransplant(From->ExternalConstraint, I,
+                                      ReasonFailed);
 }
 
 std::string

clang/lib/3C/ProgramInfo.cpp

@@ -402,87 +402,84 @@ void ProgramInfo::exitCompilationUnit() {
   return;
 }
 
-void ProgramInfo::insertIntoExternalFunctionMap(ExternalFunctionMapType &Map,
-                                                const std::string &FuncName,
-                                                FVConstraint *NewC,
-                                                FunctionDecl *FD,
-                                                ASTContext *C) {
-  if (Map.find(FuncName) == Map.end()) {
-    Map[FuncName] = NewC;
-  } else {
-    auto *OldC = Map[FuncName];
-    if (!OldC->hasBody()) {
-      if (NewC->hasBody() ||
-          (OldC->numParams() == 0 && NewC->numParams() != 0)) {
-        NewC->brainTransplant(OldC, *this);
-        Map[FuncName] = NewC;
-      } else {
-        // if the current FV constraint is not a definition?
-        // then merge.
-        std::string ReasonFailed = "";
-        OldC->mergeDeclaration(NewC, *this, ReasonFailed);
-        bool MergingFailed = ReasonFailed != "";
-        if (MergingFailed) {
-          clang::DiagnosticsEngine &DE = C->getDiagnostics();
-          unsigned MergeFailID = DE.getCustomDiagID(
-              DiagnosticsEngine::Fatal, "merging failed for %q0 due to %1");
-          const auto Pointer = reinterpret_cast<intptr_t>(FD);
-          const auto Kind =
-              clang::DiagnosticsEngine::ArgumentKind::ak_nameddecl;
-          auto DiagBuilder = DE.Report(FD->getLocation(), MergeFailID);
-          DiagBuilder.AddTaggedVal(Pointer, Kind);
-          DiagBuilder.AddString(ReasonFailed);
-        }
-        if (MergingFailed) {
-          // Kill the process and stop conversion
-          // Without this code here, 3C simply ignores this pair of functions
-          // and converts the rest of the files as it will (in semi-compliance
-          // with Mike's (2) listed on the original issue (#283)
-          exit(1);
-        }
-      }
-    } else if (NewC->hasBody()) {
-      clang::DiagnosticsEngine &DE = C->getDiagnostics();
-      unsigned DuplicateDefinitionsID = DE.getCustomDiagID(
-          DiagnosticsEngine::Fatal, "duplicate definition for function %0");
-      DE.Report(FD->getLocation(), DuplicateDefinitionsID).AddString(FuncName);
-      exit(1);
-    } else {
-      // The old constraint has a body, but we've encountered another prototype
-      // for the function.
-      assert(OldC->hasBody() && !NewC->hasBody());
-      // By transplanting the atoms of OldC into NewC, we ensure that any
-      // constraints applied to NewC later on constrain the atoms of OldC.
-      NewC->brainTransplant(OldC, *this);
+void ProgramInfo::insertNewFVConstraint(FunctionDecl *FD, FVConstraint *NewC,
+                                        ASTContext *C) {
+  std::string FuncName = FD->getNameAsString();
+
+  // Choose a storage location
+
+  // assume a global function, but change to a static if not
+  ExternalFunctionMapType *Map = &ExternalFunctionFVCons;
+  if (!FD->isGlobal()) {
+    // if the filename has not yet been seen, just insert and we're done
+    auto Psl = PersistentSourceLoc::mkPSL(FD, *C);
+    std::string FileName = Psl.getFileName();
+    if (StaticFunctionFVCons.find(FileName) == StaticFunctionFVCons.end()){
+      StaticFunctionFVCons[FileName][FuncName] = NewC;
+      return;
     }
+
+    // store in static map
+    Map = &StaticFunctionFVCons[FileName];
   }
-}
 
-void ProgramInfo::insertIntoStaticFunctionMap(StaticFunctionMapType &Map,
-                                              const std::string &FuncName,
-                                              const std::string &FileName,
-                                              FVConstraint *ToIns,
-                                              FunctionDecl *FD, ASTContext *C) {
-  if (Map.find(FileName) == Map.end())
-    Map[FileName][FuncName] = ToIns;
-  else
-    insertIntoExternalFunctionMap(Map[FileName], FuncName, ToIns, FD, C);
-}
+  // if the function has not yet been seen, just insert and we're done
+  if (Map->find(FuncName) == Map->end()) {
+    (*Map)[FuncName] = NewC;
+    return;
+  }
 
-void ProgramInfo::insertNewFVConstraint(FunctionDecl *FD, FVConstraint *FVCon,
-                                        ASTContext *C) {
-  std::string FuncName = FD->getNameAsString();
-  if (FD->isGlobal()) {
-    // external method.
-    insertIntoExternalFunctionMap(ExternalFunctionFVCons, FuncName, FVCon, FD,
-                                  C);
-  } else {
-    // static method
-    auto Psl = PersistentSourceLoc::mkPSL(FD, *C);
-    std::string FuncFileName = Psl.getFileName();
-    insertIntoStaticFunctionMap(StaticFunctionFVCons, FuncName, FuncFileName,
-                                FVCon, FD, C);
+  // Resolve conflicts
+
+  // We need to keep the version with a body, if it exists,
+  // so branch based on it
+  auto *OldC = (*Map)[FuncName];
+  bool NewHasBody = NewC->hasBody();
+  bool OldHasBody = OldC->hasBody();
+  std::string ReasonFailed = "";
+
+  if (OldHasBody && NewHasBody) {
+    // Two separate bodies for a function is irreconcilable
+    ReasonFailed = "multiple function bodies";
+  } else if (OldHasBody && !NewHasBody) {
+    // By transplanting the atoms of OldC into NewC, we ensure that any
+    // constraints applied to NewC later on constrain the atoms of OldC.
+    NewC->brainTransplant(OldC, *this, ReasonFailed);
+  } else if (!OldHasBody && NewHasBody) {
+    // as above, but the new version has the body
+    NewC->brainTransplant(OldC, *this, ReasonFailed);
+    (*Map)[FuncName] = NewC;
+  } else if (!OldHasBody && !NewHasBody) {
+    // special case for undeclared params
+    if (OldC->numParams() == 0 && NewC->numParams() != 0) {
+      NewC->brainTransplant(OldC, *this, ReasonFailed);
+      (*Map)[FuncName] = NewC;
+    } else {
+      // Merging favors the checked types.
+      // It assumes the author changed a few and missed a few.
+      OldC->mergeDeclaration(NewC, *this, ReasonFailed);
+    }
+  }
+
+  // If successful, we're done and can skip error reporting
+  if (ReasonFailed == "") return;
+
+  // Error reporting
+  { // block to force DiagBuilder destructor and emit message
+    clang::DiagnosticsEngine &DE = C->getDiagnostics();
+    unsigned FailID = DE.getCustomDiagID(DiagnosticsEngine::Fatal,
+                                         "merging failed for %q0 due to %1");
+    const auto Pointer = reinterpret_cast<intptr_t>(FD);
+    const auto Kind = clang::DiagnosticsEngine::ArgumentKind::ak_nameddecl;
+    auto DiagBuilder = DE.Report(FD->getLocation(), FailID);
+    DiagBuilder.AddTaggedVal(Pointer, Kind);
+    DiagBuilder.AddString(ReasonFailed);
   }
+  // Kill the process and stop conversion
+  // Without this code here, 3C simply ignores this pair of functions
+  // and converts the rest of the files as it will (in semi-compliance
+  // with Mike's (2) listed on the original issue (#283)
+  exit(1);
 }
 
 void ProgramInfo::specialCaseVarIntros(ValueDecl *D, ASTContext *Context) {

clang/test/3C/difftypes1a.c

@@ -0,0 +1,7 @@
+//RUN: rm -rf %t*
+//RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1b.c -- 2>%t.stderr
+//RUN: grep -q "merging failed" %t.stderr
+
+// The desired behavior in this case is to fail, so other checks are omitted
+
+_Ptr<int> foo(int, char);