[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
juliehockett marked 2 inline comments as done.
Closed by commit rL327590: [clang-tidy] Add Zircon module to clang-tidy 
(authored by juliehockett, committed by ).
Herald added subscribers: llvm-commits, klimek.

Changed prior to commit:
  https://reviews.llvm.org/D44346?vs=138470=138476#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44346

Files:
  clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/trunk/clang-tidy/zircon/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang-tools-extra/trunk/clang-tidy/zircon/TemporaryObjectsCheck.h
  clang-tools-extra/trunk/clang-tidy/zircon/ZirconTidyModule.cpp
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/zircon-temporary-objects.rst
  clang-tools-extra/trunk/docs/clang-tidy/index.rst
  clang-tools-extra/trunk/test/clang-tidy/zircon-temporary-objects.cpp

Index: clang-tools-extra/trunk/clang-tidy/zircon/ZirconTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/zircon/ZirconTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/zircon/ZirconTidyModule.cpp
@@ -0,0 +1,40 @@
+//===--- ZirconTidyModule.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "TemporaryObjectsCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace zircon {
+
+/// This module is for Zircon-specific checks.
+class ZirconModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"zircon-temporary-objects");
+  }
+};
+
+// Register the ZirconTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("zircon-module", "Adds Zircon kernel checks.");
+} // namespace zircon
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the ZirconModule.
+volatile int ZirconModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/zircon/TemporaryObjectsCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/zircon/TemporaryObjectsCheck.h
+++ clang-tools-extra/trunk/clang-tidy/zircon/TemporaryObjectsCheck.h
@@ -0,0 +1,42 @@
+//===--- TemporaryObjectsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_TEMPORARYOBJECTSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_TEMPORARYOBJECTSCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/OptionsUtils.h"
+
+namespace clang {
+namespace tidy {
+namespace zircon {
+
+/// Construction of specific temporary objects in the Zircon kernel is
+/// discouraged.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/zircon-temporary-objects.html
+class TemporaryObjectsCheck : public ClangTidyCheck {
+public:
+  TemporaryObjectsCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context),
+Names(utils::options::parseStringList(Options.get("Names", ""))) {}
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+  std::vector Names;
+};
+
+} // namespace zircon
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ZIRCON_TEMPORARYOBJECTSCHECK_H
Index: clang-tools-extra/trunk/clang-tidy/zircon/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/zircon/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/zircon/CMakeLists.txt
@@ -0,0 +1,14 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyZirconModule
+  TemporaryObjectsCheck.cpp
+  ZirconTidyModule.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  )
Index: 

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM with some minor documentation nits.




Comment at: docs/clang-tidy/checks/zircon-temporary-objects.rst:7
+Warns on construction of specific temporary objects in the Zircon kernel. 
+If the object should be flagged, the fully qualified name of the object 
+must be explicitly passed as a parameter to the check.

Nit: the docs conflate objects and types somewhat. The configuration specifies 
prohibited types, and objects of that type are the problem. How about: `If the 
object should be flagged, the fully qualified type name must be explicitly 
passed to the check.`



Comment at: docs/clang-tidy/checks/zircon-temporary-objects.rst:36-37
+
+Note that objects must be explicitly specified in order to be flagged, 
+and so objects that inherit a specified object will not be flagged.
+

Same problem here. How about: "This check matches temporary objects without 
regard for inheritance and so a prohibited base class type does not similarly 
prohibit derived class types."


https://reviews.llvm.org/D44346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 138470.
juliehockett added a comment.

Fixing a typo sorry


https://reviews.llvm.org/D44346

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  clang-tidy/zircon/CMakeLists.txt
  clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang-tidy/zircon/TemporaryObjectsCheck.h
  clang-tidy/zircon/ZirconTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/zircon-temporary-objects.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/zircon-temporary-objects.cpp

Index: test/clang-tidy/zircon-temporary-objects.cpp
===
--- /dev/null
+++ test/clang-tidy/zircon-temporary-objects.cpp
@@ -0,0 +1,109 @@
+// RUN: %check_clang_tidy %s zircon-temporary-objects %t -- \
+// RUN:   -config="{CheckOptions: [{key: zircon-temporary-objects.Names, value: 'Foo;NS::Bar'}]}" \
+// RUN:   -header-filter=.* \
+// RUN: -- -std=c++11
+
+// Should flag instances of Foo, NS::Bar.
+
+class Foo {
+public:
+  Foo() = default;
+  Foo(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+namespace NS {
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+} // namespace NS
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+int func(Foo F) { return 1; };
+
+int main() {
+  Foo F;
+  Foo *F2 = new Foo();
+  new Foo();
+  Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F3 = Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar();
+  NS::Bar();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+  int A = func(Foo());
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Foo F4(0);
+  Foo *F5 = new Foo(0);
+  new Foo(0);
+  Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F6 = Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar(0);
+  NS::Bar(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+  int B = func(Foo(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+}
+
+namespace NS {
+
+void f() {
+  Bar();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+  Bar(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+}
+
+} // namespace NS
+
+template 
+Ty make_ty() { return Ty(); }
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: creating a temporary object of type 'Foo' is prohibited
+// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+void ty_func() {
+  make_ty();
+  make_ty();
+  make_ty();
+}
+
+// Inheriting the disallowed class does not trigger the check.
+
+class Bingo : NS::Bar {}; // Not explicitly disallowed
+
+void f2() {
+  Bingo();
+}
+
+template 
+class Quux : Ty {};
+
+void f3() {
+  Quux();
+  Quux();
+}
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -75,6 +75,7 @@
relate to any particular coding style.
 ``readability-``   Checks that target readability-related issues that don't
relate to any particular coding style.
+``zircon-``Checks related to Zircon kernel coding conventions.
 == =
 
 Clang diagnostics are treated in a similar way as check diagnostics. Clang
Index: docs/clang-tidy/checks/zircon-temporary-objects.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/zircon-temporary-objects.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - zircon-temporary-objects
+
+zircon-temporary-objects
+
+
+Warns on construction of specific temporary objects in the Zircon kernel. 
+If the object should be flagged, the fully qualified name of the object 
+must be explicitly passed as a parameter to the check.
+
+For example, given the list of classes "Foo" and "NS::Bar", all of the 
+following will trigger the warning: 
+
+.. code-block:: c++
+
+  Foo();
+  Foo F = Foo();
+  func(Foo());
+
+  namespace NS {
+
+  Bar();
+
+  }
+
+With the same list, the following will not trigger the warning:
+
+.. code-block:: c++
+
+  Foo F; // Non-temporary construction okay
+  Foo F(param);			 // Non-temporary construction okay
+  Foo *F = new Foo();	   // 

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 138468.
juliehockett marked 3 inline comments as done.
juliehockett added a comment.

Updating docs.


https://reviews.llvm.org/D44346

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  clang-tidy/zircon/CMakeLists.txt
  clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang-tidy/zircon/TemporaryObjectsCheck.h
  clang-tidy/zircon/ZirconTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/zircon-temporary-objects.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/zircon-temporary-objects.cpp

Index: test/clang-tidy/zircon-temporary-objects.cpp
===
--- /dev/null
+++ test/clang-tidy/zircon-temporary-objects.cpp
@@ -0,0 +1,109 @@
+// RUN: %check_clang_tidy %s zircon-temporary-objects %t -- \
+// RUN:   -config="{CheckOptions: [{key: zircon-temporary-objects.Names, value: 'Foo;NS::Bar'}]}" \
+// RUN:   -header-filter=.* \
+// RUN: -- -std=c++11
+
+// Should flag instances of Foo, NS::Bar.
+
+class Foo {
+public:
+  Foo() = default;
+  Foo(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+namespace NS {
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+} // namespace NS
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+int func(Foo F) { return 1; };
+
+int main() {
+  Foo F;
+  Foo *F2 = new Foo();
+  new Foo();
+  Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F3 = Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar();
+  NS::Bar();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+  int A = func(Foo());
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Foo F4(0);
+  Foo *F5 = new Foo(0);
+  new Foo(0);
+  Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F6 = Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar(0);
+  NS::Bar(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+  int B = func(Foo(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+}
+
+namespace NS {
+
+void f() {
+  Bar();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+  Bar(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+}
+
+} // namespace NS
+
+template 
+Ty make_ty() { return Ty(); }
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: creating a temporary object of type 'Foo' is prohibited
+// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+void ty_func() {
+  make_ty();
+  make_ty();
+  make_ty();
+}
+
+// Inheriting the disallowed class does not trigger the check.
+
+class Bingo : NS::Bar {}; // Not explicitly disallowed
+
+void f2() {
+  Bingo();
+}
+
+template 
+class Quux : Ty {};
+
+void f3() {
+  Quux();
+  Quux();
+}
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -75,6 +75,7 @@
relate to any particular coding style.
 ``readability-``   Checks that target readability-related issues that don't
relate to any particular coding style.
+``zircon-``Checks related to Zircon kernel coding conventions.
 == =
 
 Clang diagnostics are treated in a similar way as check diagnostics. Clang
Index: docs/clang-tidy/checks/zircon-temporary-objects.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/zircon-temporary-objects.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - zircon-temporary-objects
+
+zircon-temporary-objects
+
+
+Warns on construction of specific temporary objects in the Zircon kernel. 
+If the object should be flagged, the fully qualified name of the object 
+must be explicitly passed as a parameter to the check.
+
+For example, given the list of classes "Foo" and "NS::Bar", all of the 
+following will trigger the warning: 
+
+.. code-block:: c++
+
+  Foo();
+  Foo F = Foo();
+  func(Foo());
+
+  namespace NS {
+
+  Bar();
+
+  }
+
+With the same list, the following will not trigger the warning:
+
+.. code-block:: c++
+
+  Foo F; // Non-temporary construction okay
+  Foo F(param);			 // Non-temporary 

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 138466.
juliehockett marked 2 inline comments as done.
juliehockett added a comment.

Updating decl passed to warning string


https://reviews.llvm.org/D44346

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  clang-tidy/zircon/CMakeLists.txt
  clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang-tidy/zircon/TemporaryObjectsCheck.h
  clang-tidy/zircon/ZirconTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/zircon-temporary-objects.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/zircon-temporary-objects.cpp

Index: test/clang-tidy/zircon-temporary-objects.cpp
===
--- /dev/null
+++ test/clang-tidy/zircon-temporary-objects.cpp
@@ -0,0 +1,109 @@
+// RUN: %check_clang_tidy %s zircon-temporary-objects %t -- \
+// RUN:   -config="{CheckOptions: [{key: zircon-temporary-objects.Names, value: 'Foo;NS::Bar'}]}" \
+// RUN:   -header-filter=.* \
+// RUN: -- -std=c++11
+
+// Should flag instances of Foo, NS::Bar.
+
+class Foo {
+public:
+  Foo() = default;
+  Foo(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+namespace NS {
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+} // namespace NS
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+int func(Foo F) { return 1; };
+
+int main() {
+  Foo F;
+  Foo *F2 = new Foo();
+  new Foo();
+  Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F3 = Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar();
+  NS::Bar();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+  int A = func(Foo());
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Foo F4(0);
+  Foo *F5 = new Foo(0);
+  new Foo(0);
+  Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F6 = Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar(0);
+  NS::Bar(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+  int B = func(Foo(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+}
+
+namespace NS {
+
+void f() {
+  Bar();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+  Bar(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+}
+
+} // namespace NS
+
+template 
+Ty make_ty() { return Ty(); }
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: creating a temporary object of type 'Foo' is prohibited
+// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+void ty_func() {
+  make_ty();
+  make_ty();
+  make_ty();
+}
+
+// Inheriting the disallowed class does not trigger the check.
+
+class Bingo : NS::Bar {}; // Not explicitly disallowed
+
+void f2() {
+  Bingo();
+}
+
+template 
+class Quux : Ty {};
+
+void f3() {
+  Quux();
+  Quux();
+}
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -75,6 +75,7 @@
relate to any particular coding style.
 ``readability-``   Checks that target readability-related issues that don't
relate to any particular coding style.
+``zircon-``Checks related to Zircon kernel coding conventions.
 == =
 
 Clang diagnostics are treated in a similar way as check diagnostics. Clang
Index: docs/clang-tidy/checks/zircon-temporary-objects.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/zircon-temporary-objects.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - zircon-temporary-objects
+
+zircon-temporary-objects
+
+
+Warns on construction of specific temporary objects in the Zircon kernel. 
+If the object should be flagged, the fully qualified name of the object 
+must be explicitly passed as a parameter to the check.
+
+For example, given the list of classes "Foo" and "NS::Bar", all of the 
+following will trigger the warning: 
+
+.. code-block:: c++
+
+  Foo();
+  Foo F = Foo();
+  func(Foo());
+
+  namespace NS {
+
+  Bar();
+
+  }
+
+With the same list, the following will not trigger the warning:
+
+.. code-block:: c++
+
+  Foo F; // Non-temporary construction okay
+  Foo F(param);			

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/zircon/TemporaryObjectsCheck.cpp:51
+ "creating a temporary object of type %0 is prohibited")
+<< D->getConstructor()->getParent()->getQualifiedNameAsString();
+}

aaron.ballman wrote:
> aaron.ballman wrote:
> > You can skip the call to `getQualifiedNameAsString()`; the diagnostics 
> > engine will handle it properly.
> This doesn't seem to be done?
Ah, I see why now. You should change to `%q0` (and drop the existing single 
quotes) and then get rid of `getQualifiedNameAsString()`.


https://reviews.llvm.org/D44346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tidy/fuchsia/FuchsiaTidyModule.cpp:44
+CheckFactories.registerCheck(
+"fuchsia-zx-temporary-objects");
   }

aaron.ballman wrote:
> Do we want a zircon module instead? I'm wondering about people who enable 
> modules by doing `fuchsia-*` and whether or not they would expect this check 
> (and others for zircon) to be enabled.
We definitely can -- it would be nice to have the additional separation (so 
that non-zircon parts of fuchsia don't need to explicitly disable checks.



Comment at: test/clang-tidy/zircon-temporary-objects.cpp:82
+
+} // namespace NS

aaron.ballman wrote:
> Some additional test cases:
> ```
> template 
> Ty make_ty() { return Ty{}; }
> 
> // Call make_ty<> with two types: one allowed and one disallowed; I assume 
> only the disallowed triggers the diagnostic.
> 
> struct Bingo : NS::Bar {}; // Not explicitly disallowed
> 
> void f() {
>   Bingo{}; // Should this diagnose because it inherits from Bar?
> }
> 
> // Assuming derived classes diagnose if the base is prohibited:
> template 
> struct Quux : Ty {};
> 
> void f() {
>   Quux{}; // Diagnose
>   Quux{}; // Fine?
> }
> ```
For the inheritance ones, `Quux` would have to be explicitly included in the 
list in order to be triggered by the checker (to avoid over-diagnosing), so in 
this case no warnings should be generated.



Comment at: test/clang-tidy/zircon-temporary-objects.cpp:86
+Ty make_ty() { return Ty(); }
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: creating a temporary object of 
type 'Bar' is prohibited
+// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: creating a temporary object of 
type 'Foo' is prohibited

aaron.ballman wrote:
> Why? I thought `Bar` was allowed, but `NS::Bar` was prohibited?
Artifact of passing in the decl instead of the fully-qualified name string -- 
the automatic to-string method generates just the unqualified name. 


https://reviews.llvm.org/D44346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/zircon/TemporaryObjectsCheck.cpp:51
+ "creating a temporary object of type %0 is prohibited")
+<< D->getConstructor()->getParent()->getQualifiedNameAsString();
+}

aaron.ballman wrote:
> You can skip the call to `getQualifiedNameAsString()`; the diagnostics engine 
> will handle it properly.
This doesn't seem to be done?



Comment at: docs/clang-tidy/checks/zircon-temporary-objects.rst:36-37
+
+  class Derived : Foo {} // Derived is not explicitly disallowed
+  Derived(); // and so temporary construction is okay
+

Can you hoist this into the exposition as well? It seems sufficiently important 
to warrant calling out in something other than an example.


https://reviews.llvm.org/D44346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 138392.
juliehockett marked 3 inline comments as done.
juliehockett added a comment.

Fixing tests and updating docs


https://reviews.llvm.org/D44346

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  clang-tidy/zircon/CMakeLists.txt
  clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang-tidy/zircon/TemporaryObjectsCheck.h
  clang-tidy/zircon/ZirconTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/zircon-temporary-objects.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/zircon-temporary-objects.cpp

Index: test/clang-tidy/zircon-temporary-objects.cpp
===
--- /dev/null
+++ test/clang-tidy/zircon-temporary-objects.cpp
@@ -0,0 +1,109 @@
+// RUN: %check_clang_tidy %s zircon-temporary-objects %t -- \
+// RUN:   -config="{CheckOptions: [{key: zircon-temporary-objects.Names, value: 'Foo;NS::Bar'}]}" \
+// RUN:   -header-filter=.* \
+// RUN: -- -std=c++11
+
+// Should flag instances of Foo, NS::Bar.
+
+class Foo {
+public:
+  Foo() = default;
+  Foo(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+namespace NS {
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+} // namespace NS
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+int func(Foo F) { return 1; };
+
+int main() {
+  Foo F;
+  Foo *F2 = new Foo();
+  new Foo();
+  Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F3 = Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar();
+  NS::Bar();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+  int A = func(Foo());
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Foo F4(0);
+  Foo *F5 = new Foo(0);
+  new Foo(0);
+  Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F6 = Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar(0);
+  NS::Bar(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+  int B = func(Foo(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+}
+
+namespace NS {
+
+void f() {
+  Bar();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+  Bar(0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'NS::Bar' is prohibited
+}
+
+} // namespace NS
+
+template 
+Ty make_ty() { return Ty(); }
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: creating a temporary object of type 'Foo' is prohibited
+// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: creating a temporary object of type 'NS::Bar' is prohibited
+
+void ty_func() {
+  make_ty();
+  make_ty();
+  make_ty();
+}
+
+// Inheriting the disallowed class does not trigger the check.
+
+class Bingo : NS::Bar {}; // Not explicitly disallowed
+
+void f2() {
+  Bingo();
+}
+
+template 
+class Quux : Ty {};
+
+void f3() {
+  Quux();
+  Quux();
+}
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -75,6 +75,7 @@
relate to any particular coding style.
 ``readability-``   Checks that target readability-related issues that don't
relate to any particular coding style.
+``zircon-``Checks related to Zircon kernel coding conventions.
 == =
 
 Clang diagnostics are treated in a similar way as check diagnostics. Clang
Index: docs/clang-tidy/checks/zircon-temporary-objects.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/zircon-temporary-objects.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - zircon-temporary-objects
+
+zircon-temporary-objects
+
+
+Warns on construction of specific temporary objects in the Zircon kernel. 
+If the object should be flagged, the fully qualified name of the object 
+must be explicitly passed as a parameter to the check.
+
+For example, given the list of classes "Foo" and "NS::Bar", all of the 
+following will trigger the warning: 
+
+.. code-block:: c++
+
+  Foo();
+  Foo F = Foo();
+  func(Foo());
+
+  namespace NS {
+
+  Bar();
+
+  }
+
+With the same list, the following will not trigger the warning:
+
+.. code-block:: c++
+
+  Foo F; // Non-temporary construction okay
+  Foo F(param);			 // 

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/clang-tidy/zircon-temporary-objects.cpp:86
+Ty make_ty() { return Ty(); }
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: creating a temporary object of 
type 'Bar' is prohibited
+// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: creating a temporary object of 
type 'Foo' is prohibited

Why? I thought `Bar` was allowed, but `NS::Bar` was prohibited?



Comment at: test/clang-tidy/zircon-temporary-objects.cpp:95
+
+// Inheriting the disallowed class does not trigger the check.
+

The documentation should explicitly call this out, as it's different than I 
would have expected (I was assuming the derived classes would inherit the 
prohibition).



Comment at: test/clang-tidy/zircon-temporary-objects.cpp:107-108
+void f3() {
+  Quux(); // Diagnose
+  Quux(); // Fine?
+}

The comments here seem amiss.


https://reviews.llvm.org/D44346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 138286.
juliehockett marked 3 inline comments as done.
juliehockett added a comment.

Addomg tests amd fixing documentation


https://reviews.llvm.org/D44346

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  clang-tidy/zircon/CMakeLists.txt
  clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang-tidy/zircon/TemporaryObjectsCheck.h
  clang-tidy/zircon/ZirconTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/zircon-temporary-objects.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/zircon-temporary-objects.cpp

Index: test/clang-tidy/zircon-temporary-objects.cpp
===
--- /dev/null
+++ test/clang-tidy/zircon-temporary-objects.cpp
@@ -0,0 +1,109 @@
+// RUN: %check_clang_tidy %s zircon-temporary-objects %t -- \
+// RUN:   -config="{CheckOptions: [{key: zircon-temporary-objects.Names, value: 'Foo;NS::Bar'}]}" \
+// RUN:   -header-filter=.* \
+// RUN: -- -std=c++11
+
+// Should flag instances of Foo, NS::Bar.
+
+class Foo {
+public:
+  Foo() = default;
+  Foo(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+namespace NS {
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+} // namespace NS
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+int func(Foo F) { return 1; };
+
+int main() {
+  Foo F;
+  Foo *F2 = new Foo();
+  new Foo();
+  Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F3 = Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar();
+  NS::Bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Bar' is prohibited
+
+  int A = func(Foo());
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Foo F4(0);
+  Foo *F5 = new Foo(0);
+  new Foo(0);
+  Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F6 = Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar(0);
+  NS::Bar(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Bar' is prohibited
+
+  int B = func(Foo(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+}
+
+namespace NS {
+
+void f() {
+  Bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Bar' is prohibited
+  Bar(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Bar' is prohibited
+}
+
+} // namespace NS
+
+template 
+Ty make_ty() { return Ty(); }
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: creating a temporary object of type 'Bar' is prohibited
+// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: creating a temporary object of type 'Foo' is prohibited
+
+void ty_func() {
+  make_ty();
+  make_ty();
+  make_ty();
+}
+
+// Inheriting the disallowed class does not trigger the check.
+
+class Bingo : NS::Bar {}; // Not explicitly disallowed
+
+void f2() {
+  Bingo();
+}
+
+template 
+class Quux : Ty {};
+
+void f3() {
+  Quux(); // Diagnose
+  Quux(); // Fine?
+}
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -75,6 +75,7 @@
relate to any particular coding style.
 ``readability-``   Checks that target readability-related issues that don't
relate to any particular coding style.
+``zircon-``Checks related to Zircon kernel coding conventions.
 == =
 
 Clang diagnostics are treated in a similar way as check diagnostics. Clang
Index: docs/clang-tidy/checks/zircon-temporary-objects.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/zircon-temporary-objects.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - zircon-temporary-objects
+
+zircon-temporary-objects
+
+
+Warns on construction of specific temporary objects in the Zircon kernel.
+
+For example, given the list of classes "Foo" and "NS::Bar", all of the 
+following will trigger the warning: 
+
+.. code-block:: c++
+
+  Foo();
+  Foo F = Foo();
+  func(Foo());
+
+  namespace NS {
+
+  Bar();
+
+  }
+
+With the same list, the following will not trigger the warning:
+
+.. code-block:: c++
+
+  Foo F;// Non-temporary construction okay
+  Foo F(param);			// Non-temporary construction okay
+  Foo *F = new Foo();	// New construction okay
+
+  Bar(); // Not NS::Bar, so okay
+  NS::Bar 

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I dreamed up some test cases that may or may not make sense. I think they boil 
down to a few things:

- Does inheriting from a prohibited type still diagnose when the derived type 
is used to construct a temporary?
- Should it still be prohibited if the temporary type is being returned?




Comment at: clang-tidy/zircon/ZirconTidyModule.cpp:29
+};
+// Register the ZirconTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add

Can you add a bit of vertical whitespace before this comment?



Comment at: test/clang-tidy/zircon-temporary-objects.cpp:82
+
+} // namespace NS

Some additional test cases:
```
template 
Ty make_ty() { return Ty{}; }

// Call make_ty<> with two types: one allowed and one disallowed; I assume only 
the disallowed triggers the diagnostic.

struct Bingo : NS::Bar {}; // Not explicitly disallowed

void f() {
  Bingo{}; // Should this diagnose because it inherits from Bar?
}

// Assuming derived classes diagnose if the base is prohibited:
template 
struct Quux : Ty {};

void f() {
  Quux{}; // Diagnose
  Quux{}; // Fine?
}
```


https://reviews.llvm.org/D44346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:77
 
+- New `zircon-temporary-objects
+  
`_ 
check

Please sort new checks in alphabetical order.


https://reviews.llvm.org/D44346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 138264.
juliehockett marked 6 inline comments as done.
juliehockett added a comment.

Addressing comments


https://reviews.llvm.org/D44346

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  clang-tidy/zircon/CMakeLists.txt
  clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang-tidy/zircon/TemporaryObjectsCheck.h
  clang-tidy/zircon/ZirconTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/zircon-temporary-objects.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/zircon-temporary-objects.cpp

Index: test/clang-tidy/zircon-temporary-objects.cpp
===
--- /dev/null
+++ test/clang-tidy/zircon-temporary-objects.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s zircon-temporary-objects %t -- \
+// RUN:   -config="{CheckOptions: [{key: zircon-temporary-objects.Names, value: 'Foo;NS::Bar'}]}" \
+// RUN:   -header-filter=.* \
+// RUN: -- -std=c++11
+
+// Should flag instances of Foo, NS::Bar.
+
+class Foo {
+public:
+  Foo() = default;
+  Foo(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+namespace NS {
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+} // namespace NS
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+int func(Foo F) { return 1; };
+
+int main() {
+  Foo F;
+  Foo *F2 = new Foo();
+  new Foo();
+  Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F3 = Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar();
+  NS::Bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Bar' is prohibited
+
+  int A = func(Foo());
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Foo F4(0);
+  Foo *F5 = new Foo(0);
+  new Foo(0);
+  Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Foo' is prohibited
+  Foo F6 = Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type 'Foo' is prohibited
+
+  Bar(0);
+  NS::Bar(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Bar' is prohibited
+
+  int B = func(Foo(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type 'Foo' is prohibited
+}
+
+namespace NS {
+
+void f() {
+  Bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Bar' is prohibited
+  Bar(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type 'Bar' is prohibited
+}
+
+} // namespace NS
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -75,6 +75,7 @@
relate to any particular coding style.
 ``readability-``   Checks that target readability-related issues that don't
relate to any particular coding style.
+``zircon-``Checks related to Zircon kernel coding conventions.
 == =
 
 Clang diagnostics are treated in a similar way as check diagnostics. Clang
Index: docs/clang-tidy/checks/zircon-temporary-objects.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/zircon-temporary-objects.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - zircon-temporary-objects
+
+zircon-temporary-objects
+
+
+Warns on construction of specific temporary objects in the Zircon kernel.
+
+For example, given the list of classes "Foo" and "NS::Bar", all of the 
+following will trigger the warning: 
+
+.. code-block:: c++
+
+  Foo();
+  Foo F = Foo();
+  func(Foo());
+
+  namespace NS {
+
+  Bar();
+
+  }
+
+With the same list, the following will not trigger the warning:
+
+.. code-block:: c++
+
+  Foo F;// Non-temporary construction okay
+  Foo F(param);			// Non-temporary construction okay
+  Foo *F = new Foo();	// New construction okay
+
+  Bar(); // Not NS::Bar, so okay
+  NS::Bar B;			// Non-temporary construction okay
+
+Options
+---
+
+.. option:: Names
+
+   A semi-colon-separated list of fully-qualified names of C++ classes that 
+   should not be constructed as temporaries. Default is empty.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -225,3 +225,4 @@
readability-static-definition-in-anonymous-namespace
readability-string-compare
readability-uniqueptr-delete-release
+   zircon-temporary-objects

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/tool/ClangTidyMain.cpp:527
 
-// This anchor is used to force the linker to link the GoogleModule.
+// This anchor is used to force the linker to link the FuchsiaModule.
 extern volatile int FuchsiaModuleAnchorSource;

You can go ahead and commit this change directly to trunk, no need to roll it 
in to this patch (since it's an unrelated drive-by). No review necessary.



Comment at: clang-tidy/zircon/TemporaryObjectsCheck.cpp:31
+void TemporaryObjectsCheck::registerMatchers(MatchFinder *Finder) {
+  // Matcher for default constructors
+  Finder->addMatcher(

Full stop at the end of comments (here and elsewhere).



Comment at: clang-tidy/zircon/TemporaryObjectsCheck.cpp:51
+ "creating a temporary object of type %0 is prohibited")
+<< D->getConstructor()->getParent()->getQualifiedNameAsString();
+}

You can skip the call to `getQualifiedNameAsString()`; the diagnostics engine 
will handle it properly.



Comment at: clang-tidy/zircon/TemporaryObjectsCheck.h:21-22
+/// Construction of specific temporary objects in the Zircon kernel is
+/// discouraged. Takes the list of such discouraged temporary objects as a
+/// parameter.
+///

I think the "Takes the list" sentence could be made more clear as "The list of 
disallowed temporary object types is configurable.", but you could probably 
just drop the sentence entirely as well.



Comment at: docs/clang-tidy/checks/list.rst:93
fuchsia-virtual-inheritance
+   zircon-temporary-objects
google-build-explicit-make-pair

Can you keep this list alphabetical?



Comment at: docs/clang-tidy/index.rst:78
relate to any particular coding style.
+``zircon-``Checks related to Zercon kernel coding conventions.
 == 
=

s/Zercon/Zircon


https://reviews.llvm.org/D44346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 138241.
juliehockett marked 8 inline comments as done.
juliehockett retitled this revision from "[clang-tidy] Add Fuchsia checker for 
temporary objects" to "[clang-tidy] Add Zircon module to clang-tidy".
juliehockett edited the summary of this revision.
juliehockett added a comment.

Moved check to new Zircon module, fixed comments


https://reviews.llvm.org/D44346

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  clang-tidy/zircon/CMakeLists.txt
  clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang-tidy/zircon/TemporaryObjectsCheck.h
  clang-tidy/zircon/ZirconTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/zircon-temporary-objects.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/zircon-temporary-objects.cpp

Index: test/clang-tidy/zircon-temporary-objects.cpp
===
--- /dev/null
+++ test/clang-tidy/zircon-temporary-objects.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s zircon-temporary-objects %t -- \
+// RUN:   -config="{CheckOptions: [{key: zircon-temporary-objects.Names, value: 'Foo;NS::Bar'}]}" \
+// RUN:   -header-filter=.* \
+// RUN: -- -std=c++11
+
+// Should flag instances of Foo, NS::Bar
+
+class Foo {
+public:
+  Foo() = default;
+  Foo(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+namespace NS {
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+} // namespace NS
+
+class Bar {
+public:
+  Bar() = default;
+  Bar(int Val) : Val(Val){};
+
+private:
+  int Val;
+};
+
+int func(Foo F) { return 1; };
+
+int main() {
+  Foo F;
+  Foo *F2 = new Foo();
+  new Foo();
+  Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type Foo is prohibited
+  Foo F3 = Foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type Foo is prohibited
+
+  Bar();
+  NS::Bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type NS::Bar is prohibited
+
+  int A = func(Foo());
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type Foo is prohibited
+
+  Foo F4(0);
+  Foo *F5 = new Foo(0);
+  new Foo(0);
+  Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type Foo is prohibited
+  Foo F6 = Foo(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: creating a temporary object of type Foo is prohibited
+
+  Bar(0);
+  NS::Bar(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type NS::Bar is prohibited
+
+  int B = func(Foo(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: creating a temporary object of type Foo is prohibited
+}
+
+namespace NS {
+
+void f() {
+  Bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type NS::Bar is prohibited
+  Bar(0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: creating a temporary object of type NS::Bar is prohibited
+}
+
+} // namespace NS
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -75,6 +75,7 @@
relate to any particular coding style.
 ``readability-``   Checks that target readability-related issues that don't
relate to any particular coding style.
+``zircon-``Checks related to Zercon kernel coding conventions.
 == =
 
 Clang diagnostics are treated in a similar way as check diagnostics. Clang
Index: docs/clang-tidy/checks/zircon-temporary-objects.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/zircon-temporary-objects.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - zircon-temporary-objects
+
+zircon-temporary-objects
+
+
+Warns on construction of specific temporary objects in the Zircon kernel.
+
+For example, given the list of classes "Foo" and "NS::Bar", all of the 
+following will trigger the warning: 
+
+.. code-block:: c++
+
+  Foo();
+  Foo F = Foo();
+  func(Foo());
+
+  namespace NS {
+
+  Bar();
+
+  }
+
+With the same list, the following will not trigger the warning:
+
+.. code-block:: c++
+
+  Foo F;// Non-temporary construction okay
+  Foo F(param);			// Non-temporary construction okay
+  Foo *F = new Foo();	// New construction okay
+
+  Bar(); // Not NS::Bar, so okay
+  NS::Bar B;			// Non-temporary construction okay
+
+Options
+---
+
+.. option:: Names
+
+   A semi-colon-separated list of fully-qualified names of C++ classes that 
+   should not be constructed as temporaries. Default is empty.
Index: docs/clang-tidy/checks/list.rst
===
---