[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<ZxTemporaryObjectsCheck>(
+        "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 <typename Ty>
> 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 <typename Ty>
> struct Quux : Ty {};
> 
> void f() {
>   Quux<NS::Bar>{}; // Diagnose
>   Quux<Bar>{}; // 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