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

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added a comment.

In D67649#1688376 <https://reviews.llvm.org/D67649#1688376>, @jhenderson wrote:

> LGTM, but I wouldn't mind a second opinion.


Waiting until tomorrow noon (October 2nd ,12pm BST) before committing to allow people to comment.



================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:10
 #include "llvm/Support/FileCheck.h"
+#include "../lib/Support/FileCheck.h"
 #include "gtest/gtest.h"
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67649