StreamFile subclasses Stream (from lldbUtility) and is backed by a File
(from lldbHost). It does not depend on anything from lldbCore or any of its
sibling libraries, so I think it makes sense for this to live in
lldbHost instead.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Shouldn't this be next to the other Stream class in lldbUtility ?
lldb/include/lldb/Host/StreamFile.h | ||
---|---|---|
12 | Do we really need this header ? We could forward declare FileSP do avoid having to include it here |
lldbUtility is the lowest layer and does not depend on anything in lldbHost. That being said, StreamFile uses File and FileSystem from lldbHost, so that is the lowest layer it can sit on without a layering violation.
lldb/include/lldb/Host/StreamFile.h | ||
---|---|---|
12 | We're actually using it in this header (look at GetFile()). |
lldb/include/lldb/Host/StreamFile.h | ||
---|---|---|
12 | There is already a GetFileSP we could use that instead and remove GetFile |
lldb/include/lldb/Host/StreamFile.h | ||
---|---|---|
12 | Yeah, that would be a good idea. Especially since GetFile() itself doesn't check the validity of the FileSP it has anyway... And it doesn't seem like there's an obvious way to check for validity before calling GetFile(). Is it ok if I do that in a follow-up? I don't want a revert to move the file back to lldbCore in case something goes wrong. |
lldb/include/lldb/Host/StreamFile.h | ||
---|---|---|
12 | Sure |
Do we really need this header ? We could forward declare FileSP do avoid having to include it here