Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check of header file order cannot be displayed on the client. #314

Closed
chengxinyuyyt opened this issue Mar 23, 2020 · 2 comments
Closed

Check of header file order cannot be displayed on the client. #314

chengxinyuyyt opened this issue Mar 23, 2020 · 2 comments
Labels
bug Something isn't working

Comments

@chengxinyuyyt
Copy link

1 we find that check about header files cannot be displayed on the client.

for example: [llvm-order-check] which clang-tidy can check it but clangd can not display it.

2 From the log of clangd， we also find that the line number of warning is not correct when it comes to header files.
@kadircet
Copy link
Member

I didn't get a chance to dig thoroughly but most likely place we are filtering those diags are in: https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clangd/Diagnostics.cpp#L534

which is probably because of this diag is issued before we start parsing the main file(inside ReplayPreamble). Hence StoreDiags doesn't get a BeginSource call yet, therefore doesn't have LangOpts.

@kadircet kadircet added the bug Something isn't working label Mar 24, 2020
@kadircet
Copy link
Member

kadircet commented Jul 5, 2020

sent out https://reviews.llvm.org/D83178 for review

arichardson pushed a commit to arichardson/llvm-project that referenced this issue Mar 22, 2021
@kadircet @arichardson
[clangd] Send EOF before resetting diagnostics consumer
09839d4 
Summary:
Some clang-tidy checkers, e.g. llvm-include-order can emit diagnostics
at this callback (as mentioned in the comments).

Clangd was resetting diag consumer to IgnoreDiags before sending EOF, hence we
were unable to emit diagnostics for such checkers.

This patch changes the order of that reset and preprocosser event to make sure
we emit that diag.

Fixes clangd/clangd#314.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D83178
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kadircet @chengxinyuyyt