[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