Get the correct form of the base type for multi-decl vars with unchanged

types.

Author: mattmccutchen-cci

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

@@ -258,6 +258,14 @@ class PointerVariableConstraint : public ConstraintVariable {
   static PointerVariableConstraint *
   derefPVConstraint(PointerVariableConstraint *PVC);
 
+  // Utility used by the constructor to obtain a string representation of a
+  // declaration's base type. To preserve macros, this we first try to take
+  // the type directly from source code. Where that is not possible, the type
+  // 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, ProgramInfo &Info);
+
 private:
   std::string BaseType;
   CAtoms Vars;
@@ -295,14 +303,6 @@ class PointerVariableConstraint : public ConstraintVariable {
   void addArrayAnnotations(std::stack<std::string> &ConstArrs,
                            std::deque<std::string> &EndStrs) const;
 
-  // Utility used by the constructor to obtain a string representation of a
-  // declaration's base type. To preserve macros, this we first try to take
-  // the type directly from source code. Where that is not possible, the type
-  // 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, 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
   // empty string is returned instead.

clang/include/clang/3C/RewriteUtils.h

@@ -142,6 +142,15 @@ class DComp {
 
 typedef std::set<DeclReplacement *, DComp> RSet;
 
+// Generate a string for the declaration that includes the name, type, and
+// storage qualifier (if any) but not a trailing semicolon. Honors names
+// assigned to unnamed structs by 3C. Used by DeclRewriter::rewriteMultiDecl for
+// variables that don't have a replacement and by StructVariableInitializer
+// (which appends an initializer).
+std::string mkStringForDeclWithUnchangedType(DeclaratorDecl *DD,
+                                             ASTContext &Context,
+                                             ProgramInfo &Info);
+
 // This class is used to figure out which global variables are part of
 // multi-variable declarations. For local variables, all variables in a single
 // multi declaration are grouped together in a DeclStmt object. This is not the

clang/lib/3C/DeclRewriter.cpp

@@ -419,36 +419,27 @@ void DeclRewriter::rewriteMultiDecl(DeclReplacement *N, RSet &ToRewrite,
         // When the type hasn't changed, we still need to insert the original
         // type for the variable.
 
-        // This is a bit of trickery needed to get a string representation of
-        // the declaration without the initializer. We don't want to rewrite to
-        // initializer because this causes problems when rewriting casts and
-        // generic function calls later on. (issue 267)
-        auto *VD = dyn_cast<VarDecl>(DL);
-        Expr *Init = nullptr;
-        if (VD && VD->hasInit()) {
-          Init = VD->getInit();
-          VD->setInit(nullptr);
-        }
-
-        // Dump the declaration (without the initializer) to a string. Printing
-        // the AST node gives the full declaration including the base type which
-        // is not present in the multi-decl source code.
-        std::string DeclStr = "";
-        raw_string_ostream DeclStream(DeclStr);
-        DL->print(DeclStream);
-        assert("Original decl string empty." && !DeclStream.str().empty());
+        // Note: rewriteMultiDecl is currently called only by
+        // rewriteFieldOrVarDecl with VarDecls or FieldDecls, and DeclaratorDecl
+        // is a superclass of both. When we add support for typedef multi-decls,
+        // we'll need to change this code.
+        DeclaratorDecl *DD = cast<DeclaratorDecl>(DL);
+        std::string NewDeclStr = mkStringForDeclWithUnchangedType(DD, A, Info);
+
+        // If the variable has an initializer, we want this rewrite to end
+        // before the initializer to avoid interfering with any other rewrites
+        // that 3C needs to make inside the initializer expression (issue 267).
+        // VarDecl's implementation of the getSourceRange virtual method
+        // includes the initializer, but we can manually call DeclaratorDecl's
+        // implementation, which excludes the initializer.
+        SourceLocation EndLoc = DD->DeclaratorDecl::getSourceRange().getEnd();
 
         // Do the replacement. PrevEnd is setup to be the source location of the
         // comma after the previous declaration in the multi-decl. getEndLoc is
         // either the end of the declaration or just before the initializer if
         // one is present.
-        SourceRange SR(PrevEnd, DL->getEndLoc());
-        rewriteSourceRange(R, SR, DeclStream.str());
-
-        // Undo prior trickery. This need to happen so that the PSL for the decl
-        // is not changed since the PSL is used as a map key in a few places.
-        if (VD && Init)
-          VD->setInit(Init);
+        SourceRange SR(PrevEnd, DD->DeclaratorDecl::getSourceRange().getEnd());
+        rewriteSourceRange(R, SR, NewDeclStr);
       }
     }
 

clang/lib/3C/RewriteUtils.cpp

@@ -88,6 +88,33 @@ bool DComp::operator()(DeclReplacement *Lhs, DeclReplacement *Rhs) const {
   return SM.isBeforeInTranslationUnit(X2, Y1);
 }
 
+std::string mkStringForDeclWithUnchangedType(DeclaratorDecl *DD,
+                                             ASTContext &Context,
+                                             ProgramInfo &Info) {
+  std::string TypeWithName;
+  if (isPtrOrArrayType(DD->getType())) {
+    CVarOption CVO = Info.getVariable(DD, &Context);
+    assert(CVO.hasValue() &&
+           "Missing ConstraintVariable for unchanged variable in multi-decl");
+    TypeWithName = CVO.getValue().mkString(Info.getConstraints());
+  } else {
+    // In this case, the type should just be the base type except for top-level
+    // qualifiers. (REVIEW: Can we verify that somehow?)
+    // PointerVariableConstraint::extractBaseType doesn't include qualifiers,
+    // but since we know the type is not a pointer, it should be safe to just
+    // add any qualifiers at the beginning of the string.
+    std::string QualifierPrefix = DD->getType().getQualifiers().getAsString();
+    if (!QualifierPrefix.empty())
+      QualifierPrefix += " ";
+    TypeWithName = QualifierPrefix +
+                   PointerVariableConstraint::extractBaseType(
+                       DD, DD->getTypeSourceInfo(), DD->getType(),
+                       DD->getType().getTypePtr(), Context, Info) +
+                   " " + std::string(DD->getName());
+  }
+  return getStorageQualifierString(DD) + TypeWithName;
+}
+
 void GlobalVariableGroups::addGlobalDecl(Decl *VD, std::vector<Decl *> *VDVec) {
   if (VD && GlobVarGroups.find(VD) == GlobVarGroups.end()) {
     if (VDVec == nullptr)

clang/lib/3C/StructInit.cpp

@@ -11,6 +11,7 @@
 
 #include "clang/3C/StructInit.h"
 #include "clang/3C/MappingVisitor.h"
+#include "clang/3C/RewriteUtils.h"
 #include "clang/Tooling/Transformer/SourceCode.h"
 #include <sstream>
 
@@ -58,12 +59,8 @@ void StructVariableInitializer::insertVarDecl(VarDecl *VD, DeclStmt *S) {
   bool IsVarExtern = VD->getStorageClass() == StorageClass::SC_Extern;
   if (!IsVarExtern && !VD->hasInit() && hasCheckedMembers(VD)) {
     // Create replacement declaration text with an initializer.
-    const clang::Type *Ty = VD->getType().getTypePtr();
-    std::string TQ = VD->getType().getQualifiers().getAsString();
-    if (!TQ.empty())
-      TQ += " ";
-    std::string ToReplace = getStorageQualifierString(VD) + TQ + tyToStr(Ty) +
-                            " " + VD->getName().str() + " = {}";
+    std::string ToReplace =
+        mkStringForDeclWithUnchangedType(VD, *Context, I) + " = {}";
     RewriteThese.insert(new VarDeclReplacement(VD, S, ToReplace));
   }
 }