[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers created this revision.
vabridgers added reviewers: ilya-biryukov, sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Extend test cases for tokens, and remove assertion that is unneeded and
hitting in Tokens.cpp.

Fixes Bugzilla https://bugs.llvm.org/show_bug.cgi?id=45428


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77507

Files:
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp


Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===================================================================
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -484,6 +484,57 @@
     ['EMPTY'_9, 'EMPTY_FUNC'_10) => ['<eof>'_0, '<eof>'_0)
     ['EMPTY_FUNC'_10, '<eof>'_18) => ['<eof>'_0, '<eof>'_0)
 )"},
+      // Baseline case, bugz: https://bugs.llvm.org/show_bug.cgi?id=45428
+      {R"cpp(
+    #define NUM 42
+    #define M2(a1,...) {__VA_ARGS__;}
+    #define M1(a2,...) M2(a2,__VA_ARGS__)
+    void foo(void) { M1(0, NUM); }
+    )cpp",
+       R"(expanded tokens:
+  void foo ( void ) { { 42 ; } ; }
+file './input.cpp'
+  spelled tokens:
+    # define NUM 42 # define M2 ( a1 , ... ) { __VA_ARGS__ ; } # define M1 ( a2 , ... ) M2 ( a2 , __VA_ARGS__ ) void foo ( void ) { M1 ( 0 , NUM ) ; }
+  mappings:
+    ['#'_0, 'void'_30) => ['void'_0, 'void'_0)
+    ['M1'_36, ';'_42) => ['{'_6, ';'_10)
+)"},
+      // Reproducer, bugz: https://bugs.llvm.org/show_bug.cgi?id=45428.
+      // Causes mapping miss when invoking tryConsumeSpelledUntil
+      {R"cpp(
+    #define NUM 42
+    #define M2(a1,...) {__VA_ARGS__;}
+    #define M1(a2,...) M2(a2,__VA_ARGS__)
+    #define M0 M1 
+    void foo(void) { M0(0, NUM); }
+    )cpp",
+       R"(expanded tokens:
+  void foo ( void ) { { 42 ; } ; }
+file './input.cpp'
+  spelled tokens:
+    # define NUM 42 # define M2 ( a1 , ... ) { __VA_ARGS__ ; } # define M1 ( a2 , ... ) M2 ( a2 , __VA_ARGS__ ) # define M0 M1 void foo ( void ) { M0 ( 0 , NUM ) ; }
+  mappings:
+    ['#'_0, 'void'_34) => ['void'_0, 'void'_0)
+    ['M0'_40, 'NUM'_44) => ['{'_6, ';'_10)
+    ['NUM'_44, ')'_45) => [';'_10, ';'_10)
+    [')'_45, ';'_46) => [';'_10, ';'_10)
+)"},
+      // Simplified version of new issue
+      {R"cpp(
+    #define N 42 
+    #define M0(a1) foo(a1)
+    M0(N);
+    )cpp",
+       R"(expanded tokens:
+  foo ( 42 ) ;
+file './input.cpp'
+  spelled tokens:
+    # define N 42 # define M0 ( a1 ) foo ( a1 ) M0 ( N ) ;
+  mappings:
+    ['#'_0, 'M0'_14) => ['foo'_0, 'foo'_0)
+    ['M0'_14, ';'_18) => ['foo'_0, ';'_4)
+)"},
       // File ends with a macro replacement.
       {R"cpp(
     #define FOO 10+10;
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===================================================================
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -614,10 +614,7 @@
     unsigned MappingBegin = SpelledIndex;
     ++SpelledIndex;
 
-    bool HitMapping =
-        tryConsumeSpelledUntil(File, EndOffset + 1, SpelledIndex).hasValue();
-    (void)HitMapping;
-    assert(!HitMapping && "recursive macro expansion?");
+    (void)tryConsumeSpelledUntil(File, EndOffset + 1, SpelledIndex).hasValue();
 
     TokenBuffer::Mapping M;
     M.BeginExpanded = BeginExpanded;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77507.255179.patch
Type: text/x-patch
Size: 2838 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200405/d1ce73d6/attachment.bin>