[clang-tools-extra] r368058 - Fixed toHalfOpenFileRange assertion fail
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 9 01:20:59 PDT 2019
Merged to release_90 in r368407.
On Tue, Aug 6, 2019 at 7:00 PM Shaurya Gupta via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Author: sureyeaah
> Date: Tue Aug 6 10:01:12 2019
> New Revision: 368058
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368058&view=rev
> Log:
> Fixed toHalfOpenFileRange assertion fail
>
> Summary:
> - Added new function that gets Expansion range with both ends in the same file.
> - Fixes the crash at https://github.com/clangd/clangd/issues/113
>
> Subscribers: ilya-biryukov, jkorous, arphaman, kadircet, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D65754
>
> Modified:
> clang-tools-extra/trunk/clangd/SourceCode.cpp
> clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=368058&r1=368057&r2=368058&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
> +++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Aug 6 10:01:12 2019
> @@ -296,14 +296,52 @@ static SourceRange unionTokenRange(Sourc
> E1 < E2 ? R2.getEnd() : R1.getEnd());
> }
>
> -// Returns the tokenFileRange for a given Location as a Token Range
> +// Check if two locations have the same file id.
> +static bool inSameFile(SourceLocation Loc1, SourceLocation Loc2,
> + const SourceManager &SM) {
> + return SM.getFileID(Loc1) == SM.getFileID(Loc2);
> +}
> +
> +// Find an expansion range (not necessarily immediate) the ends of which are in
> +// the same file id.
> +static SourceRange
> +getExpansionTokenRangeInSameFile(SourceLocation Loc, const SourceManager &SM,
> + const LangOptions &LangOpts) {
> + SourceRange ExpansionRange =
> + toTokenRange(SM.getImmediateExpansionRange(Loc), SM, LangOpts);
> + // Fast path for most common cases.
> + if (inSameFile(ExpansionRange.getBegin(), ExpansionRange.getEnd(), SM))
> + return ExpansionRange;
> + // Record the stack of expansion locations for the beginning, keyed by FileID.
> + llvm::DenseMap<FileID, SourceLocation> BeginExpansions;
> + for (SourceLocation Begin = ExpansionRange.getBegin(); Begin.isValid();
> + Begin = Begin.isFileID()
> + ? SourceLocation()
> + : SM.getImmediateExpansionRange(Begin).getBegin()) {
> + BeginExpansions[SM.getFileID(Begin)] = Begin;
> + }
> + // Move up the stack of expansion locations for the end until we find the
> + // location in BeginExpansions with that has the same file id.
> + for (SourceLocation End = ExpansionRange.getEnd(); End.isValid();
> + End = End.isFileID() ? SourceLocation()
> + : toTokenRange(SM.getImmediateExpansionRange(End),
> + SM, LangOpts)
> + .getEnd()) {
> + auto It = BeginExpansions.find(SM.getFileID(End));
> + if (It != BeginExpansions.end())
> + return {It->second, End};
> + }
> + llvm_unreachable(
> + "We should able to find a common ancestor in the expansion tree.");
> +}
> +// Returns the file range for a given Location as a Token Range
> // This is quite similar to getFileLoc in SourceManager as both use
> // getImmediateExpansionRange and getImmediateSpellingLoc (for macro IDs).
> // However:
> // - We want to maintain the full range information as we move from one file to
> // the next. getFileLoc only uses the BeginLoc of getImmediateExpansionRange.
> -// - We want to split '>>' tokens as the lexer parses the '>>' in template
> -// instantiations as a '>>' instead of a '>'.
> +// - We want to split '>>' tokens as the lexer parses the '>>' in nested
> +// template instantiations as a '>>' instead of two '>'s.
> // There is also getExpansionRange but it simply calls
> // getImmediateExpansionRange on the begin and ends separately which is wrong.
> static SourceRange getTokenFileRange(SourceLocation Loc,
> @@ -311,16 +349,19 @@ static SourceRange getTokenFileRange(Sou
> const LangOptions &LangOpts) {
> SourceRange FileRange = Loc;
> while (!FileRange.getBegin().isFileID()) {
> - assert(!FileRange.getEnd().isFileID() &&
> - "Both Begin and End should be MacroIDs.");
> if (SM.isMacroArgExpansion(FileRange.getBegin())) {
> - FileRange.setBegin(SM.getImmediateSpellingLoc(FileRange.getBegin()));
> - FileRange.setEnd(SM.getImmediateSpellingLoc(FileRange.getEnd()));
> + FileRange = unionTokenRange(
> + SM.getImmediateSpellingLoc(FileRange.getBegin()),
> + SM.getImmediateSpellingLoc(FileRange.getEnd()), SM, LangOpts);
> + assert(inSameFile(FileRange.getBegin(), FileRange.getEnd(), SM));
> } else {
> - SourceRange ExpansionRangeForBegin = toTokenRange(
> - SM.getImmediateExpansionRange(FileRange.getBegin()), SM, LangOpts);
> - SourceRange ExpansionRangeForEnd = toTokenRange(
> - SM.getImmediateExpansionRange(FileRange.getEnd()), SM, LangOpts);
> + SourceRange ExpansionRangeForBegin =
> + getExpansionTokenRangeInSameFile(FileRange.getBegin(), SM, LangOpts);
> + SourceRange ExpansionRangeForEnd =
> + getExpansionTokenRangeInSameFile(FileRange.getEnd(), SM, LangOpts);
> + assert(inSameFile(ExpansionRangeForBegin.getBegin(),
> + ExpansionRangeForEnd.getBegin(), SM) &&
> + "Both Expansion ranges should be in same file.");
> FileRange = unionTokenRange(ExpansionRangeForBegin, ExpansionRangeForEnd,
> SM, LangOpts);
> }
>
> Modified: clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp?rev=368058&r1=368057&r2=368058&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp (original)
> +++ clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp Tue Aug 6 10:01:12 2019
> @@ -460,15 +460,22 @@ TEST(SourceCodeTests, HalfOpenFileRange)
> #define FOO(X, Y) int Y = ++X
> #define BAR(X) X + 1
> #define ECHO(X) X
> +
> + #define BUZZ BAZZ(ADD)
> + #define BAZZ(m) m(1)
> + #define ADD(a) int f = a + 1;
> template<typename T>
> class P {};
> - void f() {
> +
> + int main() {
> $a[[P<P<P<P<P<int>>>>> a]];
> $b[[int b = 1]];
> $c[[FOO(b, c)]];
> $d[[FOO(BAR(BAR(b)), d)]];
> // FIXME: We might want to select everything inside the outer ECHO.
> ECHO(ECHO($e[[int) ECHO(e]]));
> + // Shouldn't crash.
> + $f[[BUZZ]];
> }
> )cpp");
>
> @@ -495,6 +502,7 @@ TEST(SourceCodeTests, HalfOpenFileRange)
> CheckRange("c");
> CheckRange("d");
> CheckRange("e");
> + CheckRange("f");
> }
>
> } // namespace
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list