[PATCH] D48116: [libclang] Allow skipping warnings from all included files

This revision was automatically updated to reflect the committed changes. Closed by commit rL363067: [libclang] Allow skipping warnings from all included files (authored by nik, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

yvvan accepted this revision. yvvan added a comment. This revision is now accepted and ready to land. libclang part is quite small here and looks ok. I would just accept it Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48116/new/ https://reviews.llvm.org/D48116

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

ilya-biryukov added a comment. @jkorous, could you take a look or suggest someone who can review the libclang changes? As mentioned before, I've reviewed the clang bits (which are now gone), but don't have much experience in `libclang` parts. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik added a comment. Ping. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48116/new/ https://reviews.llvm.org/D48116 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik added a comment. Ping. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48116/new/ https://reviews.llvm.org/D48116 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik added a comment. Jan? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48116/new/ https://reviews.llvm.org/D48116 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik added a comment. Sorry for the pointless ping, haven't seen the inline comments. They are addressed now. I've also increased CINDEX_VERSION_MINOR so clients can detect availability of this new flag. > It's been a while since I've looked at the ASTUnit code, though, would be > good if

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik updated this revision to Diff 199015. nik marked 2 inline comments as done. nik added a comment. Addressed inline comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48116/new/ https://reviews.llvm.org/D48116 Files: include/clang-c/Index.h

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik marked 4 inline comments as done. nik added inline comments. Herald added a subscriber: dexonsmith. Comment at: lib/Frontend/ASTUnit.cpp:682 + auto = D.getSourceManager(); + return M.isInMainFile(M.getExpansionLoc(D.getLocation())); +} ilya-biryukov

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik added a comment. Ping. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48116/new/ https://reviews.llvm.org/D48116 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

ilya-biryukov added a comment. This LGTM wrt to layering, i.e. it's nice we don't need to change the code of diagnostics. It's been a while since I've looked at the ASTUnit code, though, would be good if someone else took an extra look at it. Added a few nits too. Comment

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik updated this revision to Diff 187318. nik added a comment. Herald added a subscriber: jdoerfert. OK, filtering happens now in FilterAndStoreDiagnosticConsumer, the former StoredDiagnosticConsumer. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48116/new/

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik added a comment. Herald added a project: clang. >> For filtering in StoredDiagnosticConsumer one needs to pass the new bool >> everywhere along where "bool CaptureDiagnostics" is already passed on (to >> end up in the StoredDiagnosticConsumer constructor) . Also, >>

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

ilya-biryukov added a comment. In D48116#1322878 , @nik wrote: > In D48116#1144732 , @ilya-biryukov > wrote: > > > Have you considered doing the same filtering in ASTUnit's > > `StoredDiagnosticConsumer`? It

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik added a comment. In D48116#1144732 , @ilya-biryukov wrote: > Have you considered doing the same filtering in ASTUnit's > `StoredDiagnosticConsumer`? It should not be more difficult and allows to > avoid changing the clang's diagnostic interfaces.

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik added a comment. Herald added a subscriber: arphaman. In D48116#1144732 , @ilya-biryukov wrote: > Have you considered doing the same filtering in ASTUnit's > `StoredDiagnosticConsumer`? It should not be more difficult and allows to > avoid changing

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

yvvan added a comment. But this one misses a way to set this flag for everything except libclang. We can probably change that (Nikolai is in vacation till the end of summer so it's probably my part now) and add some tests outside of Index for it (probably Frontend) Repository: rC Clang

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

yvvan added a comment. The idea was to ignore everything including notes and remarks so that only errors from system headers are still collected. Changing consumer might be a possible way to go as well but it requires changing (or wrapping) all consumers that we need to be affected. The

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

ilya-biryukov added a comment. Sorry for the late response, was on vacation. Have you considered doing the same filtering in ASTUnit's `StoredDiagnosticConsumer`? It should not be more difficult and allows to avoid changing the clang's diagnostic interfaces. That's what we do in clangd. I

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

yvvan added a comment. @ilya-biryukov what do you think? Repository: rC Clang https://reviews.llvm.org/D48116 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik updated this revision to Diff 151116. nik added a comment. Addressed comments. Repository: rC Clang https://reviews.llvm.org/D48116 Files: include/clang-c/Index.h include/clang/Basic/Diagnostic.h lib/Basic/DiagnosticIDs.cpp test/Index/ignore-warnings-from-headers.cpp

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

yvvan added a comment. Cool! That's actually quite a small change with big outcome! Comment at: include/clang/Basic/Diagnostic.h:216 + // Suppress warnings from all included files. + bool SuppressWarningsFromIncludedFiles = false; Probably mention that it

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

nik created this revision. Herald added a subscriber: cfe-commits. Depending on the included files and the used warning flags, e.g. - Weverything, a huge number of warnings can be reported for included files. As processing that many diagnostics comes with a performance impact and not all clients