Preserving qualifiers and specifiers: |const|, |volatile|, |mutable|.

Bug: 1069567
Change-Id: Ifee67f9d96e320f6c4d0c09c1ec5a0cb2c8d0871
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146058
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760343}

tools/clang/rewrite_raw_ptr_fields/RewriteRawPtrFields.cpp

@@ -34,6 +34,7 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/LineIterator.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -52,10 +53,14 @@ class FieldDeclRewriter : public MatchFinder::MatchCallback {
 
   void run(const MatchFinder::MatchResult& result) override {
     const clang::SourceManager& source_manager = *result.SourceManager;
+
     const clang::FieldDecl* field_decl =
         result.Nodes.getNodeAs<clang::FieldDecl>("fieldDecl");
+    assert(field_decl && "matcher should bind 'fieldDecl'");
+
     const clang::TypeSourceInfo* type_source_info =
         field_decl->getTypeSourceInfo();
+    assert(type_source_info && "assuming |type_source_info| is always present");
 
     clang::QualType pointer_type = type_source_info->getType();
     assert(type_source_info->getType()->isPointerType() &&
@@ -78,27 +83,41 @@ class FieldDeclRewriter : public MatchFinder::MatchCallback {
         field_decl->getBeginLoc(),
         field_decl->getLocation().getLocWithOffset(-1));
 
+    // Calculate |replacement_text|.
+    std::string replacement_text = GenerateNewText(pointer_type);
+    if (field_decl->isMutable())
+      replacement_text.insert(0, "mutable ");
+
     // Generate and add a replacement.
     replacements_->emplace_back(
         source_manager, clang::CharSourceRange::getCharRange(replacement_range),
-        GenerateNewText(pointer_type));
+        replacement_text);
   }
 
  private:
   std::string GenerateNewText(const clang::QualType& pointer_type) {
+    std::string result;
+
     assert(pointer_type->isPointerType() && "caller must pass a pointer type!");
     clang::QualType pointee_type = pointer_type->getPointeeType();
 
+    // Preserve qualifiers.
+    assert(!pointer_type.isRestrictQualified() &&
+           "|restrict| is a C-only qualifier and CheckedPtr<T> needs C++");
+    if (pointer_type.isConstQualified())
+      result += "const ";
+    if (pointer_type.isVolatileQualified())
+      result += "volatile ";
+
     // Convert pointee type to string.
     clang::LangOptions lang_options;
     clang::PrintingPolicy printing_policy(lang_options);
     printing_policy.SuppressTagKeyword = 1;  // s/class Pointee/Pointee/
     std::string pointee_type_as_string =
         pointee_type.getAsString(printing_policy);
+    result += llvm::formatv("CheckedPtr<{0}>", pointee_type_as_string);
 
-    // TODO(lukasza): Preserve qualifiers from |pointer_type| by generating
-    // results from fresh AST (rather than via string concatenation).
-    return std::string("CheckedPtr<") + pointee_type_as_string + ">";
+    return result;
   }
 
   std::vector<Replacement>* const replacements_;

tools/clang/rewrite_raw_ptr_fields/tests/qualifiers-expected.cc

@@ -5,7 +5,7 @@
 class SomeClass;
 
 class MyClass {
-  MyClass() : ptr_field3_(nullptr), ptr_field6_(nullptr) {}
+  MyClass() : ptr_field3_(nullptr), ptr_field7_(nullptr) {}
 
   // Expected rewrite: CheckedPtr<const SomeClass> ptr_field1_;
   CheckedPtr<const SomeClass> ptr_field1_;
@@ -14,23 +14,17 @@ class MyClass {
   CheckedPtr<volatile SomeClass> ptr_field2_;
 
   // Expected rewrite: const CheckedPtr<SomeClass> ptr_field3_;
-  //
-  // TODO(lukasza): Fix this by using |qualType.getAsString|.
-  // Currently the "outer" |const| is dropped.
-  CheckedPtr<SomeClass> ptr_field3_;
+  const CheckedPtr<SomeClass> ptr_field3_;
 
   // Expected rewrite: mutable CheckedPtr<SomeClass> ptr_field4_;
-  //
-  // TODO(lukasza): Fix this by looking at |field_decl->isMutable()|.
-  // Currently the |mutable| specifier is dropped.
-  CheckedPtr<SomeClass> ptr_field4_;
+  mutable CheckedPtr<SomeClass> ptr_field4_;
 
   // Expected rewrite: CheckedPtr<const SomeClass> ptr_field5_;
   CheckedPtr<const SomeClass> ptr_field5_;
 
   // Expected rewrite: volatile CheckedPtr<const SomeClass> ptr_field6_;
-  //
-  // TODO(lukasza): Fix this by using |qualType.getAsString|.
-  // Currently the "outer" qualifiers (like |volatile| below) are dropped.
-  CheckedPtr<const SomeClass> ptr_field6_;
+  volatile CheckedPtr<const SomeClass> ptr_field6_;
+
+  // Expected rewrite: const CheckedPtr<const SomeClass> ptr_field7_;
+  const CheckedPtr<const SomeClass> ptr_field7_;
 };

tools/clang/rewrite_raw_ptr_fields/tests/qualifiers-original.cc

@@ -5,7 +5,7 @@
 class SomeClass;
 
 class MyClass {
-  MyClass() : ptr_field3_(nullptr), ptr_field6_(nullptr) {}
+  MyClass() : ptr_field3_(nullptr), ptr_field7_(nullptr) {}
 
   // Expected rewrite: CheckedPtr<const SomeClass> ptr_field1_;
   const SomeClass* ptr_field1_;
@@ -14,23 +14,17 @@ class MyClass {
   volatile SomeClass* ptr_field2_;
 
   // Expected rewrite: const CheckedPtr<SomeClass> ptr_field3_;
-  //
-  // TODO(lukasza): Fix this by using |qualType.getAsString|.
-  // Currently the "outer" |const| is dropped.
   SomeClass* const ptr_field3_;
 
   // Expected rewrite: mutable CheckedPtr<SomeClass> ptr_field4_;
-  //
-  // TODO(lukasza): Fix this by looking at |field_decl->isMutable()|.
-  // Currently the |mutable| specifier is dropped.
   mutable SomeClass* ptr_field4_;
 
   // Expected rewrite: CheckedPtr<const SomeClass> ptr_field5_;
   SomeClass const* ptr_field5_;
 
   // Expected rewrite: volatile CheckedPtr<const SomeClass> ptr_field6_;
-  //
-  // TODO(lukasza): Fix this by using |qualType.getAsString|.
-  // Currently the "outer" qualifiers (like |volatile| below) are dropped.
   const SomeClass* volatile ptr_field6_;
+
+  // Expected rewrite: const CheckedPtr<const SomeClass> ptr_field7_;
+  const SomeClass* const ptr_field7_;
 };

tools/clang/rewrite_raw_ptr_fields/tests/various-types-expected.cc

@@ -26,4 +26,4 @@ struct MyStruct {
 };
 
 // [1] non-supported-type - type that won't ever be either
-// A) allocated by PartitionAllc or B) derived from CheckedPtrSupport.
+// A) allocated by PartitionAlloc or B) derived from CheckedPtrSupport.

tools/clang/rewrite_raw_ptr_fields/tests/various-types-original.cc

@@ -26,4 +26,4 @@ struct MyStruct {
 };
 
 // [1] non-supported-type - type that won't ever be either
-// A) allocated by PartitionAllc or B) derived from CheckedPtrSupport.
+// A) allocated by PartitionAlloc or B) derived from CheckedPtrSupport.