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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Support/FileCheck.h:28
 namespace llvm {
+namespace filecheck {
 
----------------
thopre wrote:
> jhenderson wrote:
> > jhenderson wrote:
> > > dblaikie wrote:
> > > > Does this need a new namespace? We don't really use too many of them in LLVM & if things are moving out of the header, I'd expect this change would only reduce the chance of naming collisions and reduce the need for an extra namespace.
> > > I personally would prefer the namespace, as it allows simplifying the naming of the various structs everywhere within the FileCheck code, and therefore making the code more readable. I think I suggested it a while back, but it didn't happen at the time for some reason that I forget.
> > For reference, a couple of examples of namespace usage within areas of LLVM I've worked in are `object` (for the Object library) and `objcopy` (for the llvm-objcopy code).
> Forgive my naive question but why is a namespace necessary to simplfy the naming of internal types? I feel only type in the public interface would benefit from a namespace but then I'd need to come up with a new name for the FileCheck class to avoid the weird llvm::filecheck::FileCheck type.
Random related musing: If the types are really internal, why are they called "FileCheckSomeClassName"? Could they just be named "SomeClassName"?

The namespace is more for the public interface, yes, sorry for the confusion. For the FileCheck class name in that case, I'd probably go with something like "Checker" or "Rnner" since it runs the file checking code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67649