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: include/clang/Tooling/Core/Diagnostic.h:62-70 +/// Represents extra source ranges to be associated with a diagnostic. +struct DiagnosticAssociatedRanges { + DiagnosticAssociatedRanges() = default; + + DiagnosticAssociatedRanges(const SourceManager &Sources, + ArrayRef<CharSourceRange> SourceRanges); + ---------------- Why is this needed? Shouldn't `LLVM_YAML_IS_SEQUENCE_VECTOR` be enough to allow for SmallVector<DiagnosticAssociatedRange, ...> to be yaml serializable? Seems to work with `DiagnosticMessage` and `Diagnostic::Notes`. ================ Comment at: lib/Tooling/Core/Diagnostic.cpp:51 + for (const CharSourceRange &Range : SourceRanges) { + Ranges.emplace_back(DiagnosticAssociatedRange(Sources, Range)); + } ---------------- With emplace_back constructor parameters can be used directly: Ranges.emplace_back(Sources, Range); That's the whole point of `emplace_back`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/ https://reviews.llvm.org/D69782 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
