[PATCH] D93637: [libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 5 07:55:19 PST 2021


ymandel marked an inline comment as done.
ymandel added a comment.

Thanks for the review!



================
Comment at: clang/unittests/Tooling/StencilTest.cpp:273
+  std::string Snippet = R"cc(
+    Smart x;
+    x;
----------------
tdl-g wrote:
> You're only testing the "QuacksLike" case.  I suspect you should have tests that validate the "KnownSmartPointers".
> 
> Admittedly, it's a bit redundant since the known smart pointers also QuackLike pointers.  Which, I guess, raises the question of why you have the hard-coded list of KnownSmartPointers if they are covered by the QuacksLike behavior and thus can't be meaningfully tested independently.
> 
> What do you think?
You're right -- there isn't really a meaningful test specific to the known cases. I added a comment explaining that hard-coding them is an optimization. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93637/new/

https://reviews.llvm.org/D93637



More information about the cfe-commits mailing list