This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.
ClosedPublic

Authored by ioeric on Jul 3 2018, 7:03 AM.

Event Timeline

ioeric created this revision.Jul 3 2018, 7:03 AM
ilya-biryukov accepted this revision.Jul 4 2018, 2:25 AM

LGTM with a minor NIT

clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
88

NIT: Maybe use clangd's log here? To get timestamps in the output, etc.

This revision is now accepted and ready to land.Jul 4 2018, 2:25 AM
ioeric added inline comments.Jul 4 2018, 2:30 AM
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
88

The default logger doesn't seem to add timestamp? I'll keep llvm::errs() here for consistency in this file.

This revision was automatically updated to reflect the committed changes.
ilya-biryukov added inline comments.Jul 4 2018, 2:49 AM
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
88

The default logger doesn't seem to add timestamp?

Ah, yes, it's the JSONOutput that adds timestamps.... Sorry.

I'll keep llvm::errs() here for consistency in this file.

We write to stderr at the bottom of the file to show errors to the user.
This instance looks much more like a logging routine.

Since our dependencies (e.g. SymbolCollector) use logs, I'd argue it's actually more consistent across the program to use log here as well. E.g. if someone adds a LoggingSession that writes timestamps, not only SymbolCollector logs should start showing those, but this one as well.