Sema can only be used for documentation in the current file, other doc
comments should be fetched from the index.
Details
Diff Detail
- Repository
- rCTE Clang Tools Extra
- Build Status
Buildable 21569 Build 21569: arc lint + arc unit
Event Timeline
- Log on index request, remove FIXME that was addressed
- Remove SymbolID::forDecl, use existing helper instead
clangd/CodeComplete.cpp | ||
---|---|---|
755 | drive by: I think this should be vlog or dlog. |
clangd/CodeComplete.cpp | ||
---|---|---|
741 | Definitely useful. Thanks! | |
clangd/index/Index.h | ||
65 ↗ | (On Diff #160647) | Thanks for pointing this out. |
clangd/index/Index.h | ||
---|---|---|
65 ↗ | (On Diff #160647) | Yes, I think we will not add any AST-specific stuff to Index.h, that's why we have AST.h. |
clangd/CodeComplete.cpp | ||
---|---|---|
755 | Code completion also logs the number of results from sema, index, etc. using the log() call. Maybe we should turn all of them into vlog or dlog, but I'd rather leave it to a separate patch. |
clangd/CodeComplete.cpp | ||
---|---|---|
755 | I'm not sure which level code completion log messages should be in, but I think we should follow the guidelines in the logger documentation instead of the existing uses. Quote from Logger.h // log() is used for information important to understanding a clangd session. // e.g. the names of LSP messages sent are logged at this level. // This level could be enabled in production builds to allow later inspection. // vlog() is used for details often needed for debugging clangd sessions. // This level would typically be enabled for clangd developers. |
clangd/CodeComplete.cpp | ||
---|---|---|
755 | My recent experience of debugging some weird GotoDef issues tells me that log of index should be on production (since it is a non-trivial part in a clangd session), it would be really helpful to understand what is going on. |
clangd/CodeComplete.cpp | ||
---|---|---|
755 | I agree that they are useful for debugging, but they might not be interesting to end-users. And I think that's why there is vlog. Clangd developers could use a different log level with --log flag. |
clangd/CodeComplete.cpp | ||
---|---|---|
755 | I agree that vlog is probably a better fit here, but still think we should change it across CodeComplete.cpp in a single commit, rather than partially apply the guidelines to different parts of the file. However, if everyone agrees we should use vlog here, happy to use vlog too and also send a patch to update all the rest of the usages.
But if there's an agreement that we should use vlog everywhere, more than happy to change it :-) |
clangd/CodeComplete.cpp | ||
---|---|---|
755 | That sounds good to me. Thanks! |
nit: do we want to log anything here? It may be useful for debug.