vedgy added a comment.
In D139774#4041308 <https://reviews.llvm.org/D139774#4041308>, @aaron.ballman
wrote:
> Is your concern essentially that someone will add a new use to put files in a
> temp directory and not have access to this information from ASTUnit? Or do
> you have other concerns in mind?
>
> We should certainly investigate whether there are other uses of temp files
> from libclang as part of these changes. It's possible we'll find a situation
> that makes my suggestion untenable because we don't have easy access to the
> information we need.
The temporary directory could be used, now or in future, by some support code,
which is used indirectly by libclang. I found the following uses potentially
relevant to libclang:
- `Driver::GetTemporaryDirectory(StringRef Prefix)` calls
`llvm::sys::fs::createUniqueDirectory(Prefix, Path);`
- `StandardInstrumentations` => `DotCfgChangeReporter` calls
`sys::fs::createUniquePath("cfgdot-%%%%%%.dot", SV, true);`
- There are plenty of `createTemporaryFile()` uses in llvm and clang. Some of
them are likely used by libclang.
I don't know how to efficiently check whether or not each of these indirect
`system_temp_directory()` uses is in turn used by libclang. Even if they aren't
know, they might be later, when libclang API grows.
On a different note, do you think overriding temporary directory path is useful
only to libclang and not likely to be useful to any other LLVM API users?
>> Another issue is with the `FileSystemOptions` part: this class is widely
>> used in Clang. So adding a member to it could adversely affect performance
>> of unrelated code.
>
> It's good to keep an eye on that, but I'm not too worried about the overhead
> from it (I don't see uses in AST nodes). We can keep an eye on
> https://llvm-compile-time-tracker.com/ to see if there is a surprising
> compile time increase though.
[in case we pursue this design approach, which I currently don't feel is right]
Why not add a temporary path data member into `class ASTUnit` under the
`FileSystemOptions FileSystemOpts` member, not inside it? So that other code is
not burdened with unused struct member, and to be able to use a more suitable
type, like `SmallString` for the temporary path buffer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139774/new/
https://reviews.llvm.org/D139774
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
