[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

alexfh added a comment. Next step is to plumb this to clang-tidy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/ https://reviews.llvm.org/D69782 ___ cfe-commits mailing list

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

alexfh added a comment. In D69782#1886766 , @compositeprimes wrote: > I don't have commit rights (this is my first llvm commit!), so could someone > help out? Thanks! Fixed include paths and a broken test and committed as

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

This revision was automatically updated to reflect the committed changes. Closed by commit rGb26c88e3c6e0: [clang-tidy] Store all ranges in clang::tooling::Diagnostic (authored by compositeprimes, committed by alexfh). Changed prior to commit:

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

compositeprimes added a comment. I don't have commit rights (this is my first llvm commit!), so could someone help out? Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/ https://reviews.llvm.org/D69782 ___ cfe-commits

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! Do you have commit rights or should someone help you committing the patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/ https://reviews.llvm.org/D69782

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

compositeprimes added inline comments. Comment at: include/clang/Tooling/Core/Diagnostic.h:51 +/// Represents a single source ranges to be associated with a diagnostic. +struct DiagnosticAssociatedRange { + DiagnosticAssociatedRange() = default; alexfh wrote: >

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

compositeprimes updated this revision to Diff 245717. compositeprimes marked an inline comment as done. compositeprimes added a comment. Added tests for Yaml serialization CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/ https://reviews.llvm.org/D69782 Files:

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

alexfh added inline comments. Comment at: include/clang/Tooling/Core/Diagnostic.h:51 +/// Represents a single source ranges to be associated with a diagnostic. +struct DiagnosticAssociatedRange { + DiagnosticAssociatedRange() = default; This comment was lost by

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

compositeprimes added a comment. Sorry about the lack of context on the last upload, this one should have it all Comment at: include/clang/Tooling/Core/Diagnostic.h:62-70 +/// Represents extra source ranges to be associated with a diagnostic. +struct DiagnosticAssociatedRanges

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

compositeprimes updated this revision to Diff 245207. compositeprimes marked 2 inline comments as done. compositeprimes added a comment. Removed unnecessary DiagnosticAssociatedRanges struct CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/ https://reviews.llvm.org/D69782

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

alexfh added a comment. Thanks for the update! Please upload patches with full context. That makes navigating the code much easier during reviews. See https://llvm.org/docs/Phabricator.html A few more comments inline. Comment at:

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

compositeprimes updated this revision to Diff 244737. compositeprimes added a comment. Updated to serialize the ranges in yaml. This required making a few abstractions around the representation of CharSourceRange. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: include/clang/Tooling/DiagnosticsYaml.h:75 std::string BuildDirectory; +ArrayRef Ranges; }; We should serialize the

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

compositeprimes created this revision. compositeprimes added reviewers: gribozavr, alexfh. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D69782 Files: include/clang/Tooling/Core/Diagnostic.h