Clang plugin: defer checking of tag decls until after top-level decl fully parsed

It turns out that a CXXRecordDecl can change between the call to
HandleTagDeclDefinition and HandleTopLevelDecl. This happens in the
case of nested class declarations, causing us to fail to emit warnings
in some cases (see the test). See crbug.com/136863 for the full discussion.

BUG=136863
TEST=tools/clang/plugins/tests/test.sh


Review URL: https://chromiumcodereview.appspot.com/10808078

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148070 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 55 insertions, 0 deletions

diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index 49c5ce4..295471d 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -43,6 +43,20 @@ ChromeClassTester::ChromeClassTester(CompilerInstance& instance)
 ChromeClassTester::~ChromeClassTester() {}
 
 void ChromeClassTester::HandleTagDeclDefinition(TagDecl* tag) {
+  // Defer processing of this tag until its containing top-level
+  // declaration has been fully parsed. See crbug.com/136863.
+  pending_class_decls_.push_back(tag);
+}
+
+bool ChromeClassTester::HandleTopLevelDecl(DeclGroupRef D) {
+  for (size_t i = 0; i < pending_class_decls_.size(); ++i)
+    CheckTag(pending_class_decls_[i]);
+  pending_class_decls_.clear();
+
+  return true;  // true means continue parsing.
+}
+
+void ChromeClassTester::CheckTag(TagDecl* tag) {
   // We handle class types here where we have semantic information. We can only
   // check structs/classes/enums here, but we get a bunch of nice semantic
   // information instead of just parsing information.
diff --git a/tools/clang/plugins/ChromeClassTester.h b/tools/clang/plugins/ChromeClassTester.h
index f6d2e68..9dffe99 100644
--- a/tools/clang/plugins/ChromeClassTester.h
+++ b/tools/clang/plugins/ChromeClassTester.h
@@ -23,6 +23,7 @@ class ChromeClassTester : public clang::ASTConsumer {
 
   // clang::ASTConsumer:
   virtual void HandleTagDeclDefinition(clang::TagDecl* tag);
+  virtual bool HandleTopLevelDecl(clang::DeclGroupRef);
 
  protected:
   clang::CompilerInstance& instance() { return instance_; }
@@ -48,6 +49,8 @@ class ChromeClassTester : public clang::ASTConsumer {
  private:
   void BuildBannedLists();
 
+  void CheckTag(clang::TagDecl*);
+
   // Filtered versions of tags that are only called with things defined in
   // chrome header files.
   virtual void CheckChromeClass(clang::SourceLocation record_location,
@@ -75,6 +78,9 @@ class ChromeClassTester : public clang::ASTConsumer {
 
   // List of types that we don't check.
   std::set<std::string> ignored_record_names_;
+
+  // List of decls to check once the current top-level decl is parsed.
+  std::vector<clang::TagDecl*> pending_class_decls_;
 };
 
 #endif  // TOOLS_CLANG_PLUGINS_CHROMECLASSTESTER_H_
diff --git a/tools/clang/plugins/tests/nested_class_inline_ctor.cpp b/tools/clang/plugins/tests/nested_class_inline_ctor.cpp
new file mode 100644
index 0000000..aa90a95
--- /dev/null
+++ b/tools/clang/plugins/tests/nested_class_inline_ctor.cpp
@@ -0,0 +1,5 @@
+// Copyright (c) 2012 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.
+
+#include "nested_class_inline_ctor.h"
diff --git a/tools/clang/plugins/tests/nested_class_inline_ctor.h b/tools/clang/plugins/tests/nested_class_inline_ctor.h
new file mode 100644
index 0000000..01cfea9
--- /dev/null
+++ b/tools/clang/plugins/tests/nested_class_inline_ctor.h
@@ -0,0 +1,22 @@
+// Copyright (c) 2012 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.
+
+#ifndef NESTED_CLASS_INLINE_CTOR_H_
+#define NESTED_CLASS_INLINE_CTOR_H_
+
+#include <string>
+#include <vector>
+
+// See crbug.com/136863.
+
+class Foo {
+  class Bar {
+    Bar() {}
+    ~Bar() {}
+
+    std::vector<std::string> a;
+  };
+};
+
+#endif  // NESTED_CLASS_INLINE_CTOR_H_
diff --git a/tools/clang/plugins/tests/nested_class_inline_ctor.txt b/tools/clang/plugins/tests/nested_class_inline_ctor.txt
new file mode 100644
index 0000000..39bd6e1
--- /dev/null
+++ b/tools/clang/plugins/tests/nested_class_inline_ctor.txt
@@ -0,0 +1,8 @@
+In file included from nested_class_inline_ctor.cpp:5:
+./nested_class_inline_ctor.h:15:5: warning: [chromium-style] Complex constructor has an inlined body.
+    Bar() {}
+    ^
+./nested_class_inline_ctor.h:16:5: warning: [chromium-style] Complex destructor has an inline body.
+    ~Bar() {}
+    ^
+2 warnings generated.