[PATCH] D46176: Pull helper class Environment from clangFormat into libToolingCore [NFC]

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 2 05:21:35 PDT 2018


sammccall added inline comments.


================
Comment at: include/clang/Tooling/Core/Environment.h:1
+//===--- Environment.h - Sourece tooling environment --*- C++ -*-----------===//
+//
----------------
I'm not sure about this abstraction - I can't tell what it represents, and what in future should/shouldn't be added to it.

- The read API seems to be SourceManager& and FileID, and there doesn't seem to be much interaction. I think I'd usually find it easier to understand to have those two than an Environment.
- the utility to create a self-contained source manager wrapping a single virtual file is useful, but seems like that's more specialized/narrower (`struct SingleSourceManager`?)
- the fact that these virtual sourcemanagers are completely self-owning when passed as Environment seems maybe-convenient but also surprising and non-orthogonal (it's a maybe-owning-pointer by another name). Is this important somewhere?


Repository:
  rC Clang

https://reviews.llvm.org/D46176





More information about the cfe-commits mailing list