Assign record names and use them in extractBaseType.

Author: mattmccutchen-cci

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

@@ -345,7 +345,7 @@ class PointerVariableConstraint : public ConstraintVariable {
   // is regenerated from the type in the clang AST.
   static std::string extractBaseType(DeclaratorDecl *D, TypeSourceInfo *TSI,
                                      QualType QT, const Type *Ty,
-                                     const ASTContext &C);
+                                     const ASTContext &C, ProgramInfo &Info);
 
   // Try to extract string representation of the base type for a declaration
   // from the source code. If the base type cannot be extracted from source, an

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

@@ -178,6 +178,23 @@ class ProgramInfo : public ProgramVariableAdder {
   void registerTranslationUnits(
       const std::vector<std::unique_ptr<clang::ASTUnit>> &ASTs);
 
+  // Members related to automatic name generation for unnamed inline
+  // RecordDecls:
+
+  // Set of RecordDecl names already used at least once in the program, so we
+  // can avoid colliding with them.
+  std::set<std::string> UsedRecordNames;
+
+  // Map from PSL of an originally unnamed RecordDecl to the name we assigned to
+  // it, so we can ensure that names are assigned consistently when 3C naively
+  // rewrites the same header file multiple times as part of different
+  // translation units (see
+  // https://github.com/correctcomputation/checkedc-clang/issues/374#issuecomment-804283984).
+  std::map<PersistentSourceLoc, std::string> AssignedRecordNames;
+
+  void findUsedRecordNames(DeclContext *DC, ASTContext &Context);
+  void nameUnnamedRecords(DeclContext *DC, ASTContext &Context);
+
 private:
   // List of constraint variables for declarations, indexed by their location in
   // the source. This information persists across invocations of the constraint

clang/lib/3C/3C.cpp

@@ -522,6 +522,13 @@ bool _3CInterface::addVariables() {
 
   std::lock_guard<std::mutex> Lock(InterfaceMutex);
 
+  // Unnamed RecordDecl stuff.
+  // REVIEW: Move to a separate _3CInterface pass?
+  for (auto &TU : ASTs)
+    GlobalProgramInfo.findUsedRecordNames(TU->getASTContext().getTranslationUnitDecl(), TU->getASTContext());
+  for (auto &TU : ASTs)
+    GlobalProgramInfo.nameUnnamedRecords(TU->getASTContext().getTranslationUnitDecl(), TU->getASTContext());
+
   // 1. Add Variables.
   VariableAdderConsumer VA = VariableAdderConsumer(GlobalProgramInfo, nullptr);
   for (auto &TU : ASTs)

clang/lib/3C/ConstraintVariables.cpp

@@ -522,7 +522,7 @@ PointerVariableConstraint::PointerVariableConstraint(
                           TSInfo);
 
   // Get a string representing the type without pointer and array indirection.
-  BaseType = extractBaseType(D, TSInfo, QT, Ty, C);
+  BaseType = extractBaseType(D, TSInfo, QT, Ty, C, I);
 
   // check if the type is some depth of pointers to void
   // TODO: is this what the field should mean? do we want to include other
@@ -606,7 +606,25 @@ std::string PointerVariableConstraint::extractBaseType(DeclaratorDecl *D,
                                                        TypeSourceInfo *TSI,
                                                        QualType QT,
                                                        const Type *Ty,
-                                                       const ASTContext &C) {
+                                                       const ASTContext &C,
+                                                       ProgramInfo &Info) {
+  // If the base type is an unnamed RecordType to which we assigned a name, use
+  // that name.
+  const Type *UnelaboratedTy = Ty;
+  if (const ElaboratedType *ETy = dyn_cast<ElaboratedType>(Ty))
+    UnelaboratedTy = ETy->getNamedType().getTypePtr();
+  if (const RecordType *RTy = dyn_cast<RecordType>(UnelaboratedTy)) {
+    RecordDecl *RD = RTy->getDecl();
+    if (RD->getName().empty()) {
+      // REVIEW: Can this const_cast be avoided?
+      PersistentSourceLoc PSL = PersistentSourceLoc::mkPSL(RD, const_cast<ASTContext &>(C));
+      auto Iter = Info.AssignedRecordNames.find(PSL);
+      if (Iter != Info.AssignedRecordNames.end())
+        return std::string(RD->getKindName()) + " " + Iter->second;
+      // XXX: Can we get here in writable code? We should have named all unnamed RecordDecls in writable code.
+    }
+  }
+
   std::string BaseTypeStr = tryExtractBaseType(D, TSI, QT, Ty, C);
   // Fall back to rebuilding the base type based on type passed to constructor
   if (BaseTypeStr.empty())

clang/lib/3C/DeclRewriter.cpp

@@ -369,6 +369,7 @@ void DeclRewriter::rewriteMultiDecl(DeclReplacement *N, RSet &ToRewrite,
              "no other decls?");
       rewriteSourceRange(R, CharSourceRange::getCharRange(
           SameLineDecls[1]->getBeginLoc(), DL->getBeginLoc()), "");
+      // TODO: If the struct is unnamed, insert its assigned name.
     } else if (IsFirst) {
       // Rewriting the first declaration is easy. Nothing should change if its
       // type does not to be rewritten. When rewriting is required, it is

clang/lib/3C/ProgramInfo.cpp

@@ -1162,3 +1162,61 @@ void ProgramInfo::registerTranslationUnits(
     Idx++;
   }
 }
+
+void ProgramInfo::findUsedRecordNames(DeclContext *DC, ASTContext &Context) {
+  // We do our own traversal via `decls` rather than using RecursiveASTVisitor.
+  // This has the advantage of visiting RecordDecls in function parameters,
+  // which RecursiveASTVisitor doesn't do by default, though using such
+  // RecordDecls is poor practice anyway.
+  for (Decl *D : DC->decls()) {
+    if (RecordDecl *RD = dyn_cast<RecordDecl>(D)) {
+      if (!RD->getName().empty()) {
+        // There may be duplicate names if the same RecordDecl is seen in
+        // multiple translation units or different RecordDecls with the same
+        // name are used in different scopes. That is OK.
+        UsedRecordNames.insert(std::string(RD->getName()));
+      }
+    }
+    if (DeclContext *NestedDC = dyn_cast<DeclContext>(D)) {
+      findUsedRecordNames(NestedDC, Context);
+    }
+  }
+}
+
+void ProgramInfo::nameUnnamedRecords(DeclContext *DC, ASTContext &Context) {
+  for (Decl *D : DC->decls()) {
+    if (RecordDecl *RD = dyn_cast<RecordDecl>(D)) {
+      if (RD->getName().empty() && !RD->isAnonymousStructOrUnion()) {
+        PersistentSourceLoc PSL = PersistentSourceLoc::mkPSL(RD, Context);
+        if (canWrite(PSL.getFileName()) && AssignedRecordNames.find(PSL) == AssignedRecordNames.end()) {
+          std::string Basename = "unnamed";
+          // Try to name the record after a variable (or, field, typedef, etc.)
+          // in the same multi-decl that uses the record as its base type.
+          // XXX: Are there cases in which the name would be used (e.g., not for
+          // flattened records) but a suitable variable doesn't exist?
+          // REVIEW: If, by this point, we have built a data structure linking
+          // together the RecordDecl and all variables in the multi-decl, we
+          // could consult it instead of doing this ad-hoc check.
+          NamedDecl *NextDecl = cast_or_null<NamedDecl>(RD->getNextDeclInContext());
+          if (NextDecl != nullptr && !NextDecl->getName().empty() &&
+              // XXX: Wanted: Analogue of isBeforeInTranslationUnit for `<=`.
+              !Context.getSourceManager().isBeforeInTranslationUnit(RD->getBeginLoc(), NextDecl->getBeginLoc())) {
+            Basename = std::string(NextDecl->getName());
+          }
+          std::string NewName;
+          for (int Num = 1; ; Num++) {
+            NewName = Basename + "_" + std::string(RD->getKindName()) + "_" + std::to_string(Num);
+            if (UsedRecordNames.find(NewName) == UsedRecordNames.end())
+              break;
+          }
+          AssignedRecordNames.insert(std::make_pair(PSL, NewName));
+          // Don't assign the same name to another originally unnamed record.
+          UsedRecordNames.insert(NewName);
+        }
+      }
+    }
+    if (DeclContext *NestedDC = dyn_cast<DeclContext>(D)) {
+      nameUnnamedRecords(NestedDC, Context);
+    }
+  }
+}