This is an archive of the discontinued LLVM Phabricator instance.

[VirtualFileSystem] InMemoryFileSystem::addFile(): Type and Perms
ClosedPublic

Authored by benhamilton on Nov 2 2017, 3:11 PM.

Details

Summary

This implements a FIXME in InMemoryFileSystem::addFile(), allowing
clients to specify User, Group, Type, and/or Perms when creating a
file in an in-memory filesystem.

New tests included. Ran tests with:

% ninja BasicTests && ./tools/clang/unittests/Basic/BasicTests

Fixes PR#35172 (https://bugs.llvm.org/show_bug.cgi?id=35172)

Event Timeline

benhamilton created this revision.Nov 2 2017, 3:11 PM
benhamilton retitled this revision from [VirtualFileSystem] VirtualFileSystem::addFile(): Type and Perms to [VirtualFileSystem] InMemoryFileSystem::addFile(): Type and Perms.Nov 2 2017, 3:11 PM

Swap order of two lines to match params.

hokein added inline comments.Nov 3 2017, 2:07 AM
include/clang/Basic/VirtualFileSystem.h
328

Maybe use the type with default value directly, like addFile(..., uint32_t User = 0, uint32_t Group = 0, ...)? I think the arguments should be documented.

benhamilton marked an inline comment as done.Nov 3 2017, 10:05 AM
benhamilton added inline comments.
include/clang/Basic/VirtualFileSystem.h
328

If we make them default values, then clients which only care about one or two of the parameters have to copy the default values from the header file into the callsite.

For my actual use-case (testing what happens when a file is not executable), I only care about setting Perms.

That means I want to pass None for User, Group, and Type. I don't want to have to hard-code 0, 0, and sys::fs::file_type::regular_file in my callsite.

benhamilton marked an inline comment as done.

Rebase -- ping, can I get a reveiew?

hokein accepted this revision.Nov 9 2017, 3:06 AM

Sorry for missing it. The code looks good to me, but please wait @bkramer's comment before submit.

include/clang/Basic/VirtualFileSystem.h
328

OK, I see.

This revision is now accepted and ready to land.Nov 9 2017, 3:06 AM
bkramer accepted this revision.Nov 9 2017, 7:52 AM

LGTM!

This revision was automatically updated to reflect the committed changes.