[PATCH] D67649: [FileCheck] Move private interface to its own header

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:10
 #include "llvm/Support/FileCheck.h"
+#include "../lib/Support/FileCheck.h"
 #include "gtest/gtest.h"
----------------
thopre wrote:
> jhenderson wrote:
> > I don't really have a solution here, but I don't like this need for a relative include path. Are there any other examples of internal headers being used in unit tests like this? I'm guessing not, but probably simply because those headers aren't tested directly.
> TBH I didn't check at the time but yes there is 21 examples in llvm/unittests:
> 
> llvm/unittests/CodeGen/DIEHashTest.cpp
> llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
> llvm/unittests/Transforms/Vectorize/VPlanDominatorTreeTest.cpp
> llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
> llvm/unittests/Transforms/Vectorize/VPlanLoopInfoTest.cpp
> llvm/unittests/Transforms/Vectorize/VPlanPredicatorTest.cpp
> llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp
> llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
> llvm/unittests/Transforms/Vectorize/VPlanTestBase.h
> llvm/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp
> llvm/unittests/tools/llvm-cfi-verify/GraphBuilder.cpp
> llvm/unittests/tools/llvm-exegesis/ARM/AssemblerTest.cpp
> llvm/unittests/tools/llvm-exegesis/X86/AssemblerTest.cpp
> llvm/unittests/tools/llvm-exegesis/X86/SnippetGeneratorTest.cpp
> llvm/unittests/tools/llvm-exegesis/X86/SnippetRepetitorTest.cpp
> 
> I think the right solution would be to have internal headers in a separate directory tree (e.g. include/llvm/internal or something like that) but it should be done as a separate patch. I am not volunteering but if I don't forget by then I'll consider it once the whole patchset is committed.
Fair enough. Your suggestion sounds reasonable to me. I might suggest "include/llvm/detail" as that is something I've seen used in other projects (e.g. Boost).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67649