[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks, this patch raises a very good point. Having a VFS that is not overlayed over RealFS is almost always the wrong thing to do.
On the other hand, I think it's useful to have the client code mention that it overlays over real filesystem, rather than relying on magic inside ClangTool.

I suggest we add a comment hinting that vfs you pass should be overlayed over the real filesystem, unless the client code knows what it's doing.
And provide an easy-to-use helper function to do that.

  /// Creates overlayed file system with RealFS at the lowest priority and \p FS after that.
  IntrusiveRefCntPtr<vfs::FileSystem> overlayRealFs(IntrusiveRefCntPtr<vfs::FileSystem> FS);
  
  // .....
  
  /// \param FileSystem ..... 
  ///     Don't pass a vfs that is not overlayed over the RealFS unless you know what you're doing. 
  ///     Use overlayRealFS helper to easily add the RealFS overlay.



================
Comment at: include/clang/Tooling/Tooling.h:299
 public:
-  /// \brief Constructs a clang tool to run over a list of files.
+  /// \brief Constructs a Clang tool to run over a list of files.
   ///
----------------
The convention in this file seems to be not to capitalize the terms, unless they are class names.
E.g. "ClangTool" is capitalized, but "clang tool" is not.
Maybe leave the comment as is?


Repository:
  rC Clang

https://reviews.llvm.org/D45094