Page MenuHomePhabricator

Provide default virtual filesystem argument to ClangTool constructor
ClosedPublic

Authored by vladimir.plyashkun on Jan 11 2018, 5:50 AM.

Diff Detail

Repository
rC Clang

Event Timeline

vladimir.plyashkun retitled this revision from Provide default virtual filesystem argument in ClangTool constructor to Provide default virtual filesystem argument to ClangTool constructor.Jan 11 2018, 5:55 AM
ilya-biryukov added a comment.EditedJan 17 2018, 6:30 AM

Overall LGTM, but could we add a test that checks the VFS is actually used?

ilya-biryukov requested changes to this revision.Jan 17 2018, 6:30 AM
This revision now requires changes to proceed.Jan 17 2018, 6:30 AM

Implemented test-case to check that BaseFS is actually used in ClangTool

ilya-biryukov accepted this revision.Jan 18 2018, 7:25 AM

Looks good. Do you have commit access or do you need someone to land this patch for you?

This revision is now accepted and ready to land.Jan 18 2018, 7:25 AM

Looks good. Do you have commit access or do you need someone to land this patch for you?

No, i don't have commit access.

Looks good. Do you have commit access or do you need someone to land this patch for you?

No, i don't have commit access.

I can land this for you together with the other patch that you're working on.
BTW, have you seen that you could set a child revision in phabricator? That would link these two changes together, making it clear that those changes are closely related.

ilya-biryukov added inline comments.Jan 18 2018, 9:39 AM
include/clang/Tooling/Tooling.h
299

NIT: LLVM coding style requires full stop at the end of comments.
Could we also rephrase it to not mention the OverlayFileSystem, the important bit seems to be that we use it for all fs operations. Something like:

/// \param BaseFS VFS used for all underlying file accesses when running the tool.
This revision was automatically updated to reflect the committed changes.