[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: cfe/trunk/unittests/Sema/GslOwnerPointerInference.cpp:9
+
+#include "../ASTMatchers/ASTMatchersTest.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
----------------
mgehre wrote:
> thakis wrote:
> > This weird relative include path is a hint that this isn't the intended use :)
> True. But ...
> I thought about copying the `matches` function from STMatchersTest.h, but together with all callees, that's many lines of code.
> Alternatively, I could implement my own AST walking and property checking, but then the test would be much less readable.
> As yet another alternative, I though about moving the `GslOwnerPointerInference.cpp` test itself to `unittests/ASTMatchers`,
> but that would also be strange because it actually tests Sema functionality.
> 
> What would be your suggestion? (It's good that we discuss this now because I'm planning to extend this unit test further).
Do you need the full matches() function? With the current test, it's not even clear to me what exactly it tests since it looks very integration-testy and not very unit-testy. Maybe if you make the test narrower, you don't that much scaffolding? (Integration-type tests are supposed to be lit tests, and unit tests are supposed to test small details that are difficult to test via lit.)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66179