arphaman marked an inline comment as done.
arphaman added inline comments.
================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:32
+class DependencyCollectorFactory {
+public:
----------------
aganea wrote:
> arphaman wrote:
> > aganea wrote:
> > > Do you envision future uses for this factory?
> > Most likely, yes.
> >
> > I don't want to lock-in into creating the dependency printer in the
> > `PreprocessorOnlyTool`, as the eventual goal is to have it in a library,
> > and allow clients to consume modular dependencies not only using
> > `clang-scan-deps` tool itself, but using the C++ library directly
> > (potentially through a libclang C interface as well). The printer should be
> > in `clang-scan-deps` tool itself though, so it wouldn't be ideal to
> > reference it from `PreprocessorOnlyTool` now.
> My initial question was, what are the future planned factories? (and thus the
> other `DependencyCollector` implementations?)
>
> Do you think we could have the printer in the library as well?
>
> On a related note - we're already testing `clang-scan-deps` compiled as a
> DLL. Our use-case right now is to call it from each of [[
> https://github.com/fastbuild/fastbuild | Fastbuild ]]'s worker threads. We
> are using `main()` below almost as-is (except for the multithreading part
> which Fastbuild already does). The CDB JSON is passed by value from the
> Fastbuild thread, so it doesn't go through a file. I'd like to get back
> dependencies-as-a-string instead of `llvm::outs()`. The bottleneck currently
> is, as you can imagine, reading / writing files (seen in ETW traces), this
> patch should improve things a bit. I can send a sample of how we're using it
> after you commit.
> My initial question was, what are the future planned factories? (and thus the
> other DependencyCollector implementations?)
The only factory and collector right now that I have in mind is the factory
that collects the dependency list from the generator, and returns them to the
user of `PreprocessorOnlyTool`. Something like this:
```
class DependencyConsumer : public DependencyFileGenerator {
public:
class Factory : public DependencyCollectorFactory {
public:
std::shared_ptr<DependencyCollector> createDependencyCollector(
std::unique_ptr<DependencyOutputOptions> Opts) override {
return DependencyConsumer(std::move(Opts));
}
};
DependencyConsumer(std::unique_ptr<DependencyOutputOptions> Opts)
: DependencyFileGenerator(*Opts), Opts(std::move(Opts)) {}
void finishedMainFile(DiagnosticsEngine &Diags) override {
Dependencies = getDependencies();
}
private:
std::vector<std::string> Dependencies;
};
```
But the current design is not ideal for supporting that actually. I think I'll
rework it following the suggested SharedStream you mentioned earlier.
> Do you think we could have the printer in the library as well?
Yes
> On a related note - we're already testing clang-scan-deps compiled as a DLL.
> Our use-case right now is to call it from each of Fastbuild's worker threads.
> We are using main() below almost as-is (except for the multithreading part
> which Fastbuild already does).
Nice! I think the library will be a great fit for you then, the plan is to have
a service object in it which will own the shared state (like the cache of the
minimized sources), and worker objects which you can use in each thread to do
the actual work.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63579/new/
https://reviews.llvm.org/D63579
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
