[PATCH] D69648: Add VFS support for sanitizers' blacklist' 2

[Ilya Biryukov via Phabricator via llvm-commits]

ilya-biryukov added inline comments.


================
Comment at: clang/lib/Basic/SanitizerSpecialCaseList.cpp:24
+
+  for (const auto &Path : Paths) {
+    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MaybeBuffer =
----------------
jkorous wrote:
> ilya-biryukov wrote:
> > Why not pass `VFS` to the `SpecialCaseList::createInternal` instead?
> > We're duplicating the code of `createInternal` here.
> I intended to keep the scope of this patch small but open to discussion. Seems like the only other place where `SpecialCaseList` is used is `XRayFunctionFilter`. The options I see here are:
> 1. Get rid of anything filesystem-related in `SpecialCaseList` and just require buffers in the interface.
> 2. Require `VFS` in `SpecialCaseList` interface and pass it from `XRayFunctionFilter` (seems like VFS would be used just in the constructor and not in the actual methods?).
> 3. Add `VFS` to only some method/s in `SpecialCaseList`. I'd consider this a regression in interface "quality".
> 
> I'd prefer #1 and unless someone can confirm that the XRay change is trivial I'd prefer to not do #2.
I would argue the minimal change is actually #2.
```
createInternal(std::vector<string> Paths)
```
gets turned into
```
createInternal(std::vector<string> Paths, vfs::FileSystem *FS = getRealFileSystem());
```
The implementation is updated accordingly, callsites in XRay don't need an update.


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

https://reviews.llvm.org/D69648