tools/clang: Add an error if auto deduces to be a pointer type.

This patch ensures that when auto is used, it is not deduced to be a
pointer type, since that can lead to confusion when reading code.

BUG=554600

Review-Url: https://codereview.chromium.org/1691113003
Cr-Commit-Position: refs/heads/master@{#398987}

tools/clang/plugins/FindBadConstructsAction.cpp

@@ -65,6 +65,8 @@ bool FindBadConstructsAction::ParseArgs(const CompilerInstance& instance,
       options_.no_realpath = true;
     } else if (args[i] == "check-ipc") {
       options_.check_ipc = true;
+    } else if (args[i] == "check-auto-raw-pointer") {
+      options_.check_auto_raw_pointer = true;
     } else {
       parsed = false;
       llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";

tools/clang/plugins/FindBadConstructsConsumer.cpp

@@ -41,6 +41,9 @@ const char kWeakPtrFactoryOrder[] =
 const char kBadLastEnumValue[] =
     "[chromium-style] _LAST/Last constants of enum types must have the maximal "
     "value for any constant of that type.";
+const char kAutoDeducedToAPointerType[] =
+    "[chromium-style] auto variable type must not deduce to a raw pointer "
+    "type.";
 const char kNoteInheritance[] = "[chromium-style] %0 inherits from %1 here";
 const char kNoteImplicitDtor[] =
     "[chromium-style] No explicit destructor for %0 defined";
@@ -109,6 +112,21 @@ std::set<FunctionDecl*> GetLateParsedFunctionDecls(TranslationUnitDecl* decl) {
   return v.late_parsed_decls;
 }
 
+std::string GetAutoReplacementTypeAsString(QualType type) {
+  QualType non_reference_type = type.getNonReferenceType();
+  if (!non_reference_type->isPointerType())
+    return "auto";
+
+  std::string result =
+      GetAutoReplacementTypeAsString(non_reference_type->getPointeeType());
+  result += "*";
+  if (non_reference_type.isLocalConstQualified())
+    result += " const";
+  if (non_reference_type.isLocalVolatileQualified())
+    result += " volatile";
+  return result;
+}
+
 }  // namespace
 
 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance,
@@ -139,6 +157,8 @@ FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance,
       diagnostic().getCustomDiagID(getErrorLevel(), kWeakPtrFactoryOrder);
   diag_bad_enum_last_value_ =
       diagnostic().getCustomDiagID(getErrorLevel(), kBadLastEnumValue);
+  diag_auto_deduced_to_a_pointer_type_ =
+      diagnostic().getCustomDiagID(getErrorLevel(), kAutoDeducedToAPointerType);
 
   // Registers notes to make it easier to interpret warnings.
   diag_note_inheritance_ =
@@ -167,9 +187,8 @@ bool FindBadConstructsConsumer::TraverseDecl(Decl* decl) {
   return result;
 }
 
-bool FindBadConstructsConsumer::VisitDecl(clang::Decl* decl) {
-  clang::TagDecl* tag_decl = dyn_cast<clang::TagDecl>(decl);
-  if (tag_decl && tag_decl->isCompleteDefinition())
+bool FindBadConstructsConsumer::VisitTagDecl(clang::TagDecl* tag_decl) {
+  if (tag_decl->isCompleteDefinition())
     CheckTag(tag_decl);
   return true;
 }
@@ -185,6 +204,11 @@ bool FindBadConstructsConsumer::VisitCallExpr(CallExpr* call_expr) {
   return true;
 }
 
+bool FindBadConstructsConsumer::VisitVarDecl(clang::VarDecl* var_decl) {
+  CheckVarDecl(var_decl);
+  return true;
+}
+
 void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location,
                                                  CXXRecordDecl* record) {
   // By default, the clang checker doesn't check some types (templates, etc).
@@ -949,4 +973,52 @@ void FindBadConstructsConsumer::ParseFunctionTemplates(
   }
 }
 
+void FindBadConstructsConsumer::CheckVarDecl(clang::VarDecl* var_decl) {
+  if (!options_.check_auto_raw_pointer)
+    return;
+
+  // Check whether auto deduces to a raw pointer.
+  QualType non_reference_type = var_decl->getType().getNonReferenceType();
+  // We might have a case where the type is written as auto*, but the actual
+  // type is deduced to be an int**. For that reason, keep going down the
+  // pointee type until we get an 'auto' or a non-pointer type.
+  for (;;) {
+    const clang::AutoType* auto_type =
+        non_reference_type->getAs<clang::AutoType>();
+    if (auto_type) {
+      if (auto_type->isDeduced()) {
+        QualType deduced_type = auto_type->getDeducedType();
+        if (!deduced_type.isNull() && deduced_type->isPointerType() &&
+            !deduced_type->isFunctionPointerType()) {
+          // Check if we should even be considering this type (note that there
+          // should be fewer auto types than banned namespace/directory types,
+          // so check this last.
+          if (!InBannedNamespace(var_decl) &&
+              !InBannedDirectory(var_decl->getLocStart())) {
+            // The range starts from |var_decl|'s loc start, which is the
+            // beginning of the full expression defining this |var_decl|. It
+            // ends, however, where this |var_decl|'s type loc ends, since
+            // that's the end of the type of |var_decl|.
+            // Note that the beginning source location of type loc omits cv
+            // qualifiers, which is why it's not a good candidate to use for the
+            // start of the range.
+            clang::SourceRange range(
+                var_decl->getLocStart(),
+                var_decl->getTypeSourceInfo()->getTypeLoc().getLocEnd());
+            ReportIfSpellingLocNotIgnored(range.getBegin(),
+                                          diag_auto_deduced_to_a_pointer_type_)
+                << FixItHint::CreateReplacement(
+                       range,
+                       GetAutoReplacementTypeAsString(var_decl->getType()));
+          }
+        }
+      }
+    } else if (non_reference_type->isPointerType()) {
+      non_reference_type = non_reference_type->getPointeeType();
+      continue;
+    }
+    break;
+  }
+}
+
 }  // namespace chrome_checker

tools/clang/plugins/FindBadConstructsConsumer.h

@@ -50,7 +50,8 @@ class FindBadConstructsConsumer
 
   // RecursiveASTVisitor:
   bool TraverseDecl(clang::Decl* decl);
-  bool VisitDecl(clang::Decl* decl);
+  bool VisitTagDecl(clang::TagDecl* tag_decl);
+  bool VisitVarDecl(clang::VarDecl* var_decl);
   bool VisitTemplateSpecializationType(clang::TemplateSpecializationType* spec);
   bool VisitCallExpr(clang::CallExpr* call_expr);
 
@@ -105,6 +106,7 @@ class FindBadConstructsConsumer
 
   void CheckWeakPtrFactoryMembers(clang::SourceLocation record_location,
                                   clang::CXXRecordDecl* record);
+  void CheckVarDecl(clang::VarDecl* decl);
 
   void ParseFunctionTemplates(clang::TranslationUnitDecl* decl);
 
@@ -116,6 +118,7 @@ class FindBadConstructsConsumer
   unsigned diag_protected_non_virtual_dtor_;
   unsigned diag_weak_ptr_factory_order_;
   unsigned diag_bad_enum_last_value_;
+  unsigned diag_auto_deduced_to_a_pointer_type_;
   unsigned diag_note_inheritance_;
   unsigned diag_note_implicit_dtor_;
   unsigned diag_note_public_dtor_;

tools/clang/plugins/Options.h

@@ -21,6 +21,9 @@ struct Options {
   // paths. See https://crbug.com/583454 for details.
   bool no_realpath = false;
   bool check_ipc = false;
+  // Enforce that auto doesn't deduce to a raw pointer. See
+  // https://crbug.com/554600 for details.
+  bool check_auto_raw_pointer = false;
 };
 
 }  // namespace chrome_checker

tools/clang/plugins/tests/auto_raw_pointer.cpp

@@ -0,0 +1,41 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+class Foo {};
+
+void f();
+
+int main() {
+  int integer;
+  Foo foo;
+
+  auto int_copy = integer;
+  const auto const_int_copy = integer;
+  const auto& const_int_ref = integer;
+
+  auto raw_int_ptr = &integer;
+  const auto const_raw_int_ptr = &integer;
+  const auto& const_raw_int_ptr_ref = &integer;
+
+  auto* raw_int_ptr_valid = &integer;
+  const auto* const_raw_int_ptr_valid = &integer;
+
+  auto raw_foo_ptr = &foo;
+  const auto const_raw_foo_ptr = &foo;
+  const auto& const_raw_foo_ptr_ref = &foo;
+
+  auto* raw_foo_ptr_valid = &foo;
+  const auto* const_raw_foo_ptr_valid = &foo;
+
+  int *int_ptr;
+
+  auto double_ptr_auto = &int_ptr;
+  auto* double_ptr_auto_ptr = &int_ptr;
+  auto** double_ptr_auto_double_ptr = &int_ptr;
+
+  auto function_ptr = &f;
+
+  int *const *const volatile **const *pointer_awesomeness;
+  auto auto_awesome = pointer_awesomeness;
+}

tools/clang/plugins/tests/auto_raw_pointer.flags

@@ -0,0 +1 @@
+-Xclang -plugin-arg-find-bad-constructs -Xclang check-auto-raw-pointer

tools/clang/plugins/tests/auto_raw_pointer.txt

@@ -0,0 +1,37 @@
+auto_raw_pointer.cpp:17:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  auto raw_int_ptr = &integer;
+  ^~~~
+  auto*
+auto_raw_pointer.cpp:18:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  const auto const_raw_int_ptr = &integer;
+  ^~~~~~~~~~
+  auto* const
+auto_raw_pointer.cpp:19:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  const auto& const_raw_int_ptr_ref = &integer;
+  ^~~~~~~~~~~
+  auto* const
+auto_raw_pointer.cpp:24:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  auto raw_foo_ptr = &foo;
+  ^~~~
+  auto*
+auto_raw_pointer.cpp:25:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  const auto const_raw_foo_ptr = &foo;
+  ^~~~~~~~~~
+  auto* const
+auto_raw_pointer.cpp:26:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  const auto& const_raw_foo_ptr_ref = &foo;
+  ^~~~~~~~~~~
+  auto* const
+auto_raw_pointer.cpp:33:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  auto double_ptr_auto = &int_ptr;
+  ^~~~
+  auto**
+auto_raw_pointer.cpp:34:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  auto* double_ptr_auto_ptr = &int_ptr;
+  ^~~~~
+  auto**
+auto_raw_pointer.cpp:40:3: warning: [chromium-style] auto variable type must not deduce to a raw pointer type.
+  auto auto_awesome = pointer_awesomeness;
+  ^~~~
+  auto* const* const volatile** const*
+9 warnings generated.