[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-08-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG66a2e3a52564: [clangd] Send EOF before resetting diagnostics consumer (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83178/new/ https

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. As discussed offline D78038 is not going to fix header-guard-check issue, as it relies on not only the PP state, but receiving appropriate callbacks for all of the ifndef/define/endif macros, which is something we don't do in current s

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D83178#2132490 , @kadircet wrote: > Yeah concerns around header-guard checks are real (and likely to be > annoying), so will wait for D78038 . > > I suppose we can split that into multiple patc

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Yeah concerns around header-guard checks are real (and likely to be annoying), so will wait for D78038 . I suppose we can split that into multiple patches, and at least land the bit around preserving conditional stack at the end of prea

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-06 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 275627. kadircet added a comment. - Add unittest to DiagnosticsTest via llvm-include-order Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83178/new/ https://reviews.llvm.org/D83178 Files: clang-tools-extra/c

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D83178#2132034 , @njames93 wrote: > We should also disable any header-guard related checks. Currently with this > patch llvm-header-guard will fire on every header file as no macros get > marked as being used for header guar

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. We should also disable any header-guard related checks. Currently with this patch llvm-header-guard will fire on every header file as no macros get marked as being used for header guards by the pre processor. I'm guessing this is preamble related. Repository: rG LL

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:347 // FIXME: the PP callbacks skip the entire preamble. // Checks that want to see #includes in the main file do not see them. Check->registerPPCallbacks(Clang->getSourceManag

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. In D83178#2131948 , @kadircet wrote: > In D83178#2131914 , @sammccall wrote: > > > > can emit diagnostics

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D83178#2131976 , @sammccall wrote: > Maybe this originally worked and there was another change at some point that > broke it. Initial commit looks pretty broken at first glance though: https://github.com/llvm/llvm-project/

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D83178#2131914 , @sammccall wrote: > > can emit diagnostics at this callback (as mentioned in the comments). > > I'm a bit puzzled about this. The case that the code handles and the comment > refers to is if a check installs P

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. > can emit diagnostics at this callback (as mentioned in the comments). I'm a bit puzzled about this. The case that the code handles and the comment refers to is if a check installs PPCallbacks and overrides EndSourceFile. And I believe the existing code is ok for thi

[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

2020-07-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Some clang-tidy checkers, e.g. llvm-include-order can emit diagnostics at this callback (as mentioned i