From 08fcb7f3179f962394fa600becd133c58f275bf6 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar <nikolai.kosjar@theqtcompany.com> Date: Mon, 2 Nov 2015 10:39:08 +0100 Subject: [PATCH] Clang: Filter out invalid diagnostic ranges Apparently libclang might return invalid ranges. Now we discard the invalid ranges. Since there is a diagnostic location (in addition to ranges) the editor will still display an indication for the user. Task-number: QTCREATORBUG-15272 Change-Id: I351e136b9925a53fb2273a394e17873c5533798d Reviewed-by: Tobias Hunger <tobias.hunger@theqtcompany.com> Reviewed-by: Erik Verbruggen <erik.verbruggen@theqtcompany.com> --- .../clangbackend/ipcsource/diagnostic.cpp | 10 +++++---- .../clangbackend/ipcsource/sourcerange.cpp | 10 +++++++++ .../clangbackend/ipcsource/sourcerange.h | 4 ++++ .../unittest/data/diagnostic_source_range.cpp | 4 ++++ tests/unit/unittest/sourcerangetest.cpp | 21 +++++++++++++++++++ 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/tools/clangbackend/ipcsource/diagnostic.cpp b/src/tools/clangbackend/ipcsource/diagnostic.cpp index f0220e79581..e851a7c730e 100644 --- a/src/tools/clangbackend/ipcsource/diagnostic.cpp +++ b/src/tools/clangbackend/ipcsource/diagnostic.cpp @@ -106,13 +106,15 @@ DiagnosticSeverity Diagnostic::severity() const std::vector<SourceRange> Diagnostic::ranges() const { std::vector<SourceRange> ranges; - const uint rangesCount = clang_getDiagnosticNumRanges(cxDiagnostic); - ranges.reserve(rangesCount); - for (uint index = 0; index < rangesCount; ++index) - ranges.push_back(SourceRange(clang_getDiagnosticRange(cxDiagnostic, index))); + for (uint index = 0; index < rangesCount; ++index) { + const SourceRange sourceRange(clang_getDiagnosticRange(cxDiagnostic, index)); + + if (sourceRange.isValid()) + ranges.push_back(SourceRange(clang_getDiagnosticRange(cxDiagnostic, index))); + } return ranges; } diff --git a/src/tools/clangbackend/ipcsource/sourcerange.cpp b/src/tools/clangbackend/ipcsource/sourcerange.cpp index 4ea26ba1fed..7375988b2bd 100644 --- a/src/tools/clangbackend/ipcsource/sourcerange.cpp +++ b/src/tools/clangbackend/ipcsource/sourcerange.cpp @@ -39,6 +39,16 @@ SourceRange::SourceRange() { } +bool SourceRange::isNull() const +{ + return clang_Range_isNull(cxSourceRange); +} + +bool SourceRange::isValid() const +{ + return !isNull() && start().offset() < end().offset(); +} + SourceLocation SourceRange::start() const { return SourceLocation(clang_getRangeStart(cxSourceRange)); diff --git a/src/tools/clangbackend/ipcsource/sourcerange.h b/src/tools/clangbackend/ipcsource/sourcerange.h index cec9f6fa658..3c80e4e040d 100644 --- a/src/tools/clangbackend/ipcsource/sourcerange.h +++ b/src/tools/clangbackend/ipcsource/sourcerange.h @@ -44,6 +44,10 @@ class SourceRange public: SourceRange(); + + bool isNull() const; + bool isValid() const; + SourceLocation start() const; SourceLocation end() const; diff --git a/tests/unit/unittest/data/diagnostic_source_range.cpp b/tests/unit/unittest/data/diagnostic_source_range.cpp index ae0ee72f593..121d402ea89 100644 --- a/tests/unit/unittest/data/diagnostic_source_range.cpp +++ b/tests/unit/unittest/data/diagnostic_source_range.cpp @@ -7,3 +7,7 @@ int function(XXX i) { i + 20; } + +struct Foo { + someIdentifierLeadingToInvalidRange; +}; diff --git a/tests/unit/unittest/sourcerangetest.cpp b/tests/unit/unittest/sourcerangetest.cpp index 00a57d5d247..df9c4a37099 100644 --- a/tests/unit/unittest/sourcerangetest.cpp +++ b/tests/unit/unittest/sourcerangetest.cpp @@ -85,9 +85,24 @@ protected: translationUnits}; DiagnosticSet diagnosticSet{translationUnit.diagnostics()}; Diagnostic diagnostic{diagnosticSet.front()}; + Diagnostic diagnosticWithFilteredOutInvalidRange{diagnosticSet.at(1)}; ::SourceRange sourceRange{diagnostic.ranges().front()}; }; +TEST_F(SourceRange, IsNull) +{ + ::SourceRange sourceRange; + + ASSERT_TRUE(sourceRange.isNull()); +} + +TEST_F(SourceRange, IsNotNull) +{ + ::SourceRange sourceRange = diagnostic.ranges()[0]; + + ASSERT_FALSE(sourceRange.isNull()); +} + TEST_F(SourceRange, Size) { ASSERT_THAT(diagnostic.ranges().size(), 2); @@ -108,4 +123,10 @@ TEST_F(SourceRange, End) 6u, 44u)); } + +TEST_F(SourceRange, InvalidRangeIsFilteredOut) +{ + ASSERT_TRUE(diagnosticWithFilteredOutInvalidRange.ranges().empty()); +} + } -- GitLab