Page MenuHomePhabricator

[VFS] Update uses of getVFSFromYAML to use the new getVFSFromYAMLs API
AbandonedPublic

Authored by bnbarham on Mar 10 2022, 3:38 PM.

Details

Summary

Includes two test fixes (since chained mappings are no longer allowed)
and adds a new test for multiple overlays.

Uses other than CompilerInvocation.cpp are simple 1:1 mappings, but
without the need to read into a buffer first.

Depends on D121425

Diff Detail

Event Timeline

bnbarham created this revision.Mar 10 2022, 3:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 10 2022, 3:38 PM
bnbarham requested review of this revision.Mar 10 2022, 3:38 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptMar 10 2022, 3:38 PM
bnbarham added inline comments.Mar 10 2022, 3:55 PM
llvm/include/llvm/Support/Error.h
1284

Should this be in a change all by itself?

bnbarham updated this revision to Diff 414718.Mar 11 2022, 11:43 AM

Handle empty overlay file in clang tidy

bnbarham updated this revision to Diff 414751.Mar 11 2022, 2:20 PM
bnbarham edited the summary of this revision. (Show Details)

Update to single review dependency

Includes two test fixes (since chained mappings are no longer allowed)
and adds a new test for multiple overlays.

Are we sure no one needs to support chained mappings? Has there been a clang-dev discourse discussion about it already? Just concerned that some vendor might rely on being able to support this.

llvm/include/llvm/Support/Error.h
1284

Yes, but I also know you already split this out so I guess you just need to rebase :).

Includes two test fixes (since chained mappings are no longer allowed)
and adds a new test for multiple overlays.

Are we sure no one needs to support chained mappings? Has there been a clang-dev discourse discussion about it already? Just concerned that some vendor might rely on being able to support this.

I'm not *positive*, no, but I would be fairly surprised. You could just add the A -> C mapping if you really do want it. But I can start up that conversation if you think it needs having.

I actually didn't initially realise that there was a test for this case - vfsroot-with-overlay.c did test "indirection", but I completely missed it when I was looking through. I *thought* the only case was the one Nathan added in directory.c (and in that case what we really wanted was was what's now fallback).

@vsapsai do you know any clients of the chaining/nesting/indirection?

bnbarham marked an inline comment as done.Mar 11 2022, 10:53 PM
bnbarham added inline comments.
llvm/include/llvm/Support/Error.h
1284

Heh, when you suggested I pull the rename out I basically thought "welll, that answer this question"

So yep, it's out and I'll rebase this one :). Thanks!

bnbarham marked an inline comment as done.Mar 15 2022, 3:48 PM

Merged into D121425 instead.

bnbarham abandoned this revision.Mar 15 2022, 3:49 PM