[Lexer] Fix an off-by-one bug in Lexer::getAsCharRange() - NFC.

[NoQ]

Summary

As the unit test demonstrates, the .getLocWithOffset(-1) part was unnecessary.

@rsmith - you introduced these functions, WDYT?, hope i got the intent right.

The only user of this function was the plist file emitter (in Static Analyzer and ARCMigrator). It means that a lot of Static Analyzer's plist arrows are in fact off by one character. The patch carefully preserves this completely incorrect behavior and causes no functional change, i.e. no plist format breakage.

[NoQ]

I guess i'll commit, given that i don't know anybody else who uses these functions anyway.

[NoQ]

Reverted in rC357827 because clang-tidy was using this function in some of its checks. I'll have a look at if it is also affected by the bug or was already using a workaround.

[NoQ]

Namely, http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/46376 :

Running ['clang-tidy', '/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp', '-fix', '--checks=-*,google-readability-namespace-comments', '--', '--std=c++11', '-nostdinc++']...
------------------------ clang-tidy output -----------------------
3 warnings generated.
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:2: warning: namespace 'n3' not terminated with a closing comment [google-readability-namespace-comments]
}}
 ^
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:2: note: FIX-IT applied suggested code changes
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:4:11: note: namespace 'n3' starts here
namespace n3 {
          ^
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:3: warning: namespace 'n1::n2 {' not terminated with a closing comment [google-readability-namespace-comments]
}}
  ^
    // namespace n1::n2 {
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:3: note: FIX-IT applied suggested code changes
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:3:11: note: namespace 'n1::n2 {' starts here
namespace n1::n2 {
          ^
clang-tidy applied 2 of 2 suggested fixes.
Suppressed 1 warnings (1 with check filters).

------------------------------------------------------------------
------------------------------ Fixes -----------------------------
--- /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp.orig	2019-04-05 15:03:02.741836756 -0700
+++ /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp	2019-04-05 15:03:02.753836825 -0700
@@ -11,7 +11,8 @@
 //
 //
 //
-}}
+}  // namespace n3
+}  // namespace n1::n2 {
 //
 //
 

------------------------------------------------------------------
FileCheck failed:
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/google-readability-nested-namespace-comments.cpp:12:20: error: CHECK-MESSAGES: expected string not found in input
// CHECK-MESSAGES: :[[@LINE+2]]:3: warning: namespace 'n1::n2' not terminated with a closing comment [google-readability-namespace-comments]
                   ^
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp.msg:7:1: note: scanning from here
namespace n3 {
^
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp.msg:7:1: note: with expression "@LINE+2" equal to "14"
namespace n3 {
^
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp.msg:9:191: note: possible intended match here
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/google-readability-nested-namespace-comments.cpp.tmp.cpp:14:3: warning: namespace 'n1::n2 {' not terminated with a closing comment [google-readability-namespace-comments]

FAIL: Extra Tools Unit Tests :: clangd/./ClangdTests/SelectionTest.CommonAncestor (19592 of 48320)
******************** TEST 'Extra Tools Unit Tests :: clangd/./ClangdTests/SelectionTest.CommonAncestor' FAILED ********************
Note: Google Test filter = SelectionTest.CommonAncestor
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from SelectionTest
[ RUN      ] SelectionTest.CommonAncestor
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 2:25-2:29
To be equal to: Test.range()
      Which is: 2:25-2:28

            struct AAA { struct BBB { static int ccc(); };};
            int x = AAA::[[B^B^B]]::ccc();
          
TranslationUnitDecl 
  VarDecl int x = AAA::BBB::ccc()
    CallExpr AAA::BBB::ccc()
      ImplicitCastExpr AAA::BBB::ccc
        DeclRefExpr AAA::BBB::ccc
         .TypeLoc AAA::BBB

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 2:25-2:29
To be equal to: Test.range()
      Which is: 2:25-2:28

            struct AAA { struct BBB { static int ccc(); };};
            int x = AAA::[[B^BB^]]::ccc();
          
TranslationUnitDecl 
  VarDecl int x = AAA::BBB::ccc()
    CallExpr AAA::BBB::ccc()
      ImplicitCastExpr AAA::BBB::ccc
        DeclRefExpr AAA::BBB::ccc
         .TypeLoc AAA::BBB

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 2:20-2:34
To be equal to: Test.range()
      Which is: 2:20-2:33

            struct AAA { struct BBB { static int ccc(); };};
            int x = [[AAA::BBB::c^c^c]]();
          
TranslationUnitDecl 
  VarDecl int x = AAA::BBB::ccc()
    CallExpr AAA::BBB::ccc()
      ImplicitCastExpr AAA::BBB::ccc
       .DeclRefExpr AAA::BBB::ccc

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 2:20-2:36
To be equal to: Test.range()
      Which is: 2:20-2:35

            struct AAA { struct BBB { static int ccc(); };};
            int x = [[AAA::BBB::cc^c(^)]];
          
TranslationUnitDecl 
  VarDecl int x = AAA::BBB::ccc()
   .CallExpr AAA::BBB::ccc()
      ImplicitCastExpr AAA::BBB::ccc
       .DeclRefExpr AAA::BBB::ccc

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 1:25-1:55
To be equal to: Test.range()
      Which is: 1:25-1:54

            void foo() { [[if (1^11) { return; } else {^ }]] }
          
TranslationUnitDecl 
  FunctionDecl void foo()
    CompoundStmt {
    if (111) {
        return;
    } else {
    }
}

     .IfStmt if (111) {
    return;
} else {
}

        ImplicitCastExpr 111
         .IntegerLiteral 111
       *CompoundStmt {
    return;
}

         *ReturnStmt return;

       .CompoundStmt {
}


/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 3:39-3:43
To be equal to: Test.range()
      Which is: 3:39-3:42

            void foo();
            #define CALL_FUNCTION(X) X()
            void bar() { CALL_FUNCTION([[f^o^o]]); }
          
TranslationUnitDecl 
  FunctionDecl void bar()
    CompoundStmt {
    foo();
}

      CallExpr foo()
        ImplicitCastExpr foo
         .DeclRefExpr foo

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 3:39-3:43
To be equal to: Test.range()
      Which is: 3:39-3:42

            void foo();
            #define CALL_FUNCTION(X) X()
            void bar() { CALL_FUNC^TION([[fo^o]]); }
          
TranslationUnitDecl 
  FunctionDecl void bar()
    CompoundStmt {
    foo();
}

      CallExpr foo()
        ImplicitCastExpr foo
         .DeclRefExpr foo

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 3:23-4:0
To be equal to: Test.range()
      Which is: 3:23-3:46

            void foo();
            #define CALL_FUNCTION(X) X()
            void bar() [[{ C^ALL_FUNC^TION(foo); }]]
          
TranslationUnitDecl 
  FunctionDecl void bar()
   .CompoundStmt {
    foo();
}


/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 0:13-0:17
To be equal to: Test.range()
      Which is: 0:13-0:16
void foo() { [[^foo]](); }
TranslationUnitDecl 
  FunctionDecl void foo()
    CompoundStmt {
    foo();
}

      CallExpr foo()
        ImplicitCastExpr foo
         .DeclRefExpr foo

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 0:13-0:17
To be equal to: Test.range()
      Which is: 0:13-0:16
void foo() { [[f^oo]](); }
TranslationUnitDecl 
  FunctionDecl void foo()
    CompoundStmt {
    foo();
}

      CallExpr foo()
        ImplicitCastExpr foo
         .DeclRefExpr foo

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 0:13-0:17
To be equal to: Test.range()
      Which is: 0:13-0:16
void foo() { [[fo^o]](); }
TranslationUnitDecl 
  FunctionDecl void foo()
    CompoundStmt {
    foo();
}

      CallExpr foo()
        ImplicitCastExpr foo
         .DeclRefExpr foo

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 0:13-0:19
To be equal to: Test.range()
      Which is: 0:13-0:18
void foo() { [[foo^()]]; }
TranslationUnitDecl 
  FunctionDecl void foo()
    CompoundStmt {
    foo();
}

     .CallExpr foo()

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 0:13-0:17
To be equal to: Test.range()
      Which is: 0:13-0:16
void foo() { [[foo^]] (); }
TranslationUnitDecl 
  FunctionDecl void foo()
    CompoundStmt {
    foo();
}

      CallExpr foo()
        ImplicitCastExpr foo
         .DeclRefExpr foo

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 0:0-0:5
To be equal to: Test.range()
      Which is: 0:0-0:4
[[^void]] foo();
TranslationUnitDecl 
  FunctionDecl void foo()
    TypeLoc void ()
     .TypeLoc void

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 0:13-0:17
To be equal to: Test.range()
      Which is: 0:13-0:16
void foo() { [[foo^^]] (); }
TranslationUnitDecl 
  FunctionDecl void foo()
    CompoundStmt {
    foo();
}

      CallExpr foo()
        ImplicitCastExpr foo
         .DeclRefExpr foo

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 0:35-0:37
To be equal to: Test.range()
      Which is: 0:35-0:36
template <typename T> void foo() { [[^T]] t; }
TranslationUnitDecl 
  FunctionTemplateDecl template <typename T> void foo()
    FunctionDecl void foo()
      CompoundStmt {
    T t;
}

        DeclStmt T t;

          VarDecl T t
           *TypeLoc T

/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/unittests/clangd/SelectionTests.cpp:207: Failure
      Expected: nodeRange(T.commonAncestor(), AST)
      Which is: 2:22-2:61
To be equal to: Test.range()
      Which is: 2:22-2:60

            template <class T> struct Foo {};
            template <[[template<class> class /*cursor here*/^U]]>
             struct Foo<U<int>*> {};
          
TranslationUnitDecl 
  ClassTemplatePartialSpecializationDecl template <template <class > class U> struct Foo<<int> *> {}
   .TemplateTemplateParmDecl template <class > class U

[  FAILED  ] SelectionTest.CommonAncestor (258 ms)
[----------] 1 test from SelectionTest (258 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (258 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SelectionTest.CommonAncestor

 1 FAILED TEST
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209712 for file /clangd-test/TestTU.cpp
Preamble for file /clangd-test/TestTU.cpp cannot be reused. Attempting to rebuild it.
Built preamble of size 209728 for file /clangd-test/TestTU.cpp

********************

Failing Tests (3):
    Clang Tools :: clang-tidy/google-readability-nested-namespace-comments.cpp
    Extra Tools Unit Tests :: clangd/./ClangdTests/SelectionTest.CommonAncestor
    Extra Tools Unit Tests :: clangd/./ClangdTests/SelectionTest.Selected

(also clangd tests; and they seem to be manually adding +1 as a workaround)

[alexfh]

It looks like there's a number of users of this function beyond what you've mentioned:
clang-tidy/readability/IsolateDeclarationCheck.cpp:210
clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:43
clang-tidy/readability/NamespaceCommentCheck.cpp:106
clang/lib/ARCMigrate/PlistReporter.cpp:110
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:192

Could you take a brief look at those as well?

cfe/trunk/include/clang/Basic/PlistSupport.h

134 ↗

(On Diff #193974)

What kind of backwards compatibility are you trying to achieve? Where would the version skew come from?

[NoQ]

In D59977#1458051, @alexfh wrote:

It looks like there's a number of users of this function beyond what you've mentioned:
clang-tidy/readability/IsolateDeclarationCheck.cpp:210

This one uses only char-ranges, not token-ranges, so it's unaffected by the change.

In D59977#1458051, @alexfh wrote:

clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:43

They're using this function for counting the number of ::s in a::b::c::...::l, and for them an off-by-one doesn't affect the answer.

In D59977#1458051, @alexfh wrote:

clang-tidy/readability/NamespaceCommentCheck.cpp:106

Yup, that's one of the failing tests. They were expecting their CharSourceRange for x::y { (which starts with token x and ends with token {) to not include token {. I had to manually rtrim() it from the string. It sounds as if CharSourceRange is indeed supposed to be "inclusive" (include its last token), so the new behavior is correct:

229 /// The underlying SourceRange can either specify the starting/ending character
230 /// of the range, or it can specify the start of the range and the start of the
231 /// last token of the range (a "token range").  In the token range case, the
232 /// size of the last token must be measured to determine the actual end of the
233 /// range.
234 class CharSourceRange {

In D59977#1458051, @alexfh wrote:

clang/lib/ARCMigrate/PlistReporter.cpp:110
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:192

These are addressed all together by the hack in PlistSupport.h.

---

Also fixed SelectionTests; they were artificially adding 1 to the resulting range and i removed it.

I believe it should all work now.

[alexfh]

LG with a comment.

clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp

110–111 ↗

(On Diff #196145)

This all seems rather hacky. I believe, this will fail miserably when there are comments before the {. Can you leave a FIXME to rewrite this logic in terms of tokens instead?

[NoQ]

Add comments that getAsCharRange() is in fact used often misused.

[NoQ]

clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp

42 ↗

(On Diff #196310)

Added a similar comment here because this also seems flaky.

[NoQ]

clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp

110–111 ↗

(On Diff #196145)

I wonder why did people have to do this in the first place. Like, isn't such information supposed to be available in the AST?

include/clang/Basic/PlistSupport.h

 
 inline void EmitRange(raw_ostream &o, const SourceManager &SM,
                       CharSourceRange R, const FIDMap &FM, unsigned indent) {
   if (R.isInvalid()) return;
 
   assert(R.isCharRange() && "cannot handle a token range");
   Indent(o, indent) << "<array>\n";
   EmitLocation(o, SM, R.getBegin(), FM, indent + 1);
-  EmitLocation(o, SM, R.getEnd(), FM, indent + 1);
+
+  // The ".getLocWithOffset(-1)" emulates the behavior of an off-by-one bug
+  // in Lexer that is already fixed. It is here for backwards compatibility
+  // even though it is incorrect.
+  EmitLocation(o, SM, R.getEnd().getLocWithOffset(-1), FM, indent + 1);
   Indent(o, indent) << "</array>\n";
 }
 
 } // namespace markup
 } // namespace clang
 
 #endif // LLVM_CLANG_BASIC_PLISTSUPPORT_H

include/clang/Lex/Lexer.h

public:
   /// components that don't have access to the lexer and thus can't find the
   /// end of the range for themselves.
   static CharSourceRange getAsCharRange(SourceRange Range,
                                         const SourceManager &SM,
                                         const LangOptions &LangOpts) {
     SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts);
     return End.isInvalid() ? CharSourceRange()
                            : CharSourceRange::getCharRange(
-                                 Range.getBegin(), End.getLocWithOffset(-1));
+                                 Range.getBegin(), End);
   }
   static CharSourceRange getAsCharRange(CharSourceRange Range,
                                         const SourceManager &SM,
                                         const LangOptions &LangOpts) {
     return Range.isTokenRange()
                ? getAsCharRange(Range.getAsRange(), SM, LangOpts)
                : Range;
   }

unittests/Lex/LexerTest.cpp

TEST_F(LexerTest, StringizingRasString) {
   EXPECT_EQ(String1, R"(foo\n    {\"bar\":[]}\n    baz)");
   EXPECT_EQ(String2, R"(foo\n    {\"bar\":[]}\n    baz)");
   EXPECT_EQ(String3, R"(\\\n    \\n\n    \\\\n\n    \\\\)");
   EXPECT_EQ(String4, R"(\\\n    \\n\n    \\\\n\n    \\\\)");
   EXPECT_EQ(String5, R"(a\\\n\n\n    \\\\b)");
   EXPECT_EQ(String6, R"(a\\\n\n\n    \\\\b)");
 }
 
+TEST_F(LexerTest, CharRangeOffByOne) {
+  std::vector<Token> toks = Lex(R"(#define MOO 1
+    void foo() { MOO; })");
+  const Token &moo = toks[5];
+
+  EXPECT_EQ(getSourceText(moo, moo), "MOO");
+
+  SourceRange R{moo.getLocation(), moo.getLocation()};
+
+  EXPECT_TRUE(
+      Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts));
+  EXPECT_TRUE(
+      Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts));
+
+  CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts);
+
+  EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO".
+}
+
 } // anonymous namespace