subject:"\[PATCH\] D121550\: \[clang\-format\] Fix crash on invalid requires expression"

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-15 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

In D121550#3381903 , @owenpan wrote:

> Nvm. See https://github.com/llvm/llvm-project/issues/54384.

Phew. I thought I needed a fix for the fix, for the fix, for the fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121550/new/

https://reviews.llvm.org/D121550

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-15 Thread Owen Pan via Phabricator via cfe-commits

owenpan added a comment.

Nvm. See https://github.com/llvm/llvm-project/issues/54384.




Comment at: clang/lib/Format/TokenAnnotator.cpp:216
 FormatToken  = *CurrentToken->Previous;
 assert(OpeningParen.is(tok::l_paren));
 FormatToken *PrevNonComment = OpeningParen.getPreviousNonComment();

owenpan wrote:
> This patch either uncovers or causes an assertion failure here on a bunch of 
> files in `clang/test`.
I was wrong! Please ignore it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121550/new/

https://reviews.llvm.org/D121550

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-14 Thread Owen Pan via Phabricator via cfe-commits

owenpan added a comment.

After applying this patch, we got a different assertion failure on a bunch of 
files in `clang/test`, e.g.:

  ~/llvm-project/clang$ for f in $(find . -name \*.cpp -o -name \*.c) ; do 
clang-format $f > /dev/null ; done
  Assertion failed: (OpeningParen.is(tok::l_paren)), function parseParens, file 
TokenAnnotator.cpp, line 216.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
  Stack dump:
  0.Program arguments: cf ./test/Driver/ignore-xcoff-visibility.cpp
  ...




Comment at: clang/lib/Format/TokenAnnotator.cpp:216
 FormatToken  = *CurrentToken->Previous;
 assert(OpeningParen.is(tok::l_paren));
 FormatToken *PrevNonComment = OpeningParen.getPreviousNonComment();

This patch either uncovers or causes an assertion failure here on a bunch of 
files in `clang/test`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121550/new/

https://reviews.llvm.org/D121550

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-14 Thread Björn Schäpers via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGacd17a2be81a: [clang-format] Fix crash on invalid requires 
expression (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121550/new/

https://reviews.llvm.org/D121550

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -356,6 +356,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24041,9 +24041,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -421,8 +421,8 @@
   if (CurrentToken->isOneOf(tok::r_square, tok::r_brace))
 return false;
 
-  if (CurrentToken->is(tok::l_brace))
-OpeningParen.setType(TT_Unknown); // Not TT_ObjCBlockLParen
+  if (CurrentToken->is(tok::l_brace) && 
OpeningParen.is(TT_ObjCBlockLParen))
+OpeningParen.setType(TT_Unknown);
   if (CurrentToken->is(tok::comma) && CurrentToken->Next &&
   !CurrentToken->Next->HasUnescapedNewline &&
   !CurrentToken->Next->isTrailingComment())


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -356,6 +356,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24041,9 +24041,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -421,8 +421,8 @@
   if (CurrentToken->isOneOf(tok::r_square, tok::r_brace))
 return false;
 
-  if (CurrentToken->is(tok::l_brace))

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-13 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added a comment.

LGTM. And thanks for splitting the patch.




Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:335
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "

HazardyKnusperkeks wrote:
> curdeius wrote:
> > I'd prefer a valid C++ code (if it's not too complicated) if possible but 
> > that's acceptable.
> I thought I'd stick to the bug report. The only thing that comes to my mind 
> does compile on gcc, but clang gives a specific error (which I support, 
> without having looked into the standard). https://gcc.godbolt.org/z/WvT9861b6
Ok, now I see that it's actual test code. It's perfectly fine the.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121550/new/

https://reviews.llvm.org/D121550

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-13 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:215
+assert(CurrentToken->Previous && "Unknown previous token");
+FormatToken  = *CurrentToken->Previous;
+assert(OpeningParen.is(tok::l_paren));

curdeius wrote:
> Maybe you can commit the rename as a NFC commit and rebase the patch to 
> remove the noise?
Done in D121557



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:335
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "

curdeius wrote:
> I'd prefer a valid C++ code (if it's not too complicated) if possible but 
> that's acceptable.
I thought I'd stick to the bug report. The only thing that comes to my mind 
does compile on gcc, but clang gives a specific error (which I support, without 
having looked into the standard). https://gcc.godbolt.org/z/WvT9861b6


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121550/new/

https://reviews.llvm.org/D121550

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-13 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks updated this revision to Diff 414956.
HazardyKnusperkeks marked 2 inline comments as done.
HazardyKnusperkeks added a comment.

Split rename and bug fix


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121550/new/

https://reviews.llvm.org/D121550

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -331,6 +331,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23891,9 +23891,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -421,8 +421,8 @@
   if (CurrentToken->isOneOf(tok::r_square, tok::r_brace))
 return false;
 
-  if (CurrentToken->is(tok::l_brace))
-OpeningParen.setType(TT_Unknown); // Not TT_ObjCBlockLParen
+  if (CurrentToken->is(tok::l_brace) && 
OpeningParen.is(TT_ObjCBlockLParen))
+OpeningParen.setType(TT_Unknown);
   if (CurrentToken->is(tok::comma) && CurrentToken->Next &&
   !CurrentToken->Next->HasUnescapedNewline &&
   !CurrentToken->Next->isTrailingComment())


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -331,6 +331,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23891,9 +23891,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -421,8 +421,8 @@
   if (CurrentToken->isOneOf(tok::r_square, tok::r_brace))
 return false;
 
-  if (CurrentToken->is(tok::l_brace))
-OpeningParen.setType(TT_Unknown); // Not

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-13 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius accepted this revision.
curdeius added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/TokenAnnotator.cpp:215
+assert(CurrentToken->Previous && "Unknown previous token");
+FormatToken  = *CurrentToken->Previous;
+assert(OpeningParen.is(tok::l_paren));

Maybe you can commit the rename as a NFC commit and rebase the patch to remove 
the noise?



Comment at: clang/unittests/Format/FormatTest.cpp:23894-23896
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",

HazardyKnusperkeks wrote:
> I know we don't like changing tests, but I think there was a bug before which 
> were never hit in our tests but here.
> 
> Before
> ```
>  M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=130 Name=decltype L=42 PPK=2 
> FakeLParens=5/ FakeRParens=0 II=0xf136c40 Text='decltype'
>  M=0 C=0 T=Unknown S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=43 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=1 T=LambdaLSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=44 PPK=2 
> FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=45 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text=']'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=46 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=47 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text=')'
>  M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='->'
>  M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=54 PPK=2 
> FakeLParens= FakeRParens=0 II=0xf130c60 Text='std'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=coloncolon L=56 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='::'
>  M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=540 Name=identifier L=65 PPK=2 
> FakeLParens= FakeRParens=0 II=0xf130c88 Text='true_type'
>  M=0 C=0 T=LambdaLBrace S=1 F=0 B=0 BK=1 P=43 Name=l_brace L=67 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='{'
>  M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='}'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=81 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=82 PPK=2 
> FakeLParens= FakeRParens=1 II=0x0 Text=')'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=83 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text=')'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=85 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='::'
>  M=0 C=1 T=TrailingAnnotation S=0 F=0 B=0 BK=0 P=520 Name=identifier L=90 
> PPK=2 FakeLParens= FakeRParens=0 II=0xf130ca8 Text='value'
>  M=0 C=1 T=BinaryOperator S=1 F=0 B=0 BK=0 P=25 Name=ampamp L=93 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='&&'
> ```
> after
> ```
>  M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=130 Name=decltype L=42 PPK=2 
> FakeLParens=5/ FakeRParens=0 II=0xee66c40 Text='decltype'
>  M=0 C=0 T=TypeDeclarationParen S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=43 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=LambdaLSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=44 PPK=2 
> FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=45 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text=']'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=46 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=47 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text=')'
>  M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='->'
>  M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=54 PPK=2 
> FakeLParens= FakeRParens=0 II=0xee60c60 Text='std'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=coloncolon L=56 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='::'
>  M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=540 Name=identifier L=65 PPK=2 
> FakeLParens= FakeRParens=0 II=0xee60c88 Text='true_type'
>  M=0 C=0 T=LambdaLBrace S=1 F=0 B=0 BK=1 P=43 Name=l_brace L=67 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='{'
>  M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='}'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=81 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='('
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=82 PPK=2 
> FakeLParens= FakeRParens=1 II=0x0 Text=')'
>  M=0 C=0 T=TypeDeclarationParen S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=83 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text=')'
>  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=85 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='::'
>  M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=520

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-13 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:424-425
 
-  if (CurrentToken->is(tok::l_brace))
-Left->setType(TT_Unknown); // Not TT_ObjCBlockLParen
+  if (CurrentToken->is(tok::l_brace) && 
OpeningParen.is(TT_ObjCBlockLParen))
+OpeningParen.setType(TT_Unknown);
   if (CurrentToken->is(tok::comma) && CurrentToken->Next &&

This is the actual fix. It reset to many types, although the comment said what 
it should reset.



Comment at: clang/unittests/Format/FormatTest.cpp:23894-23896
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",

I know we don't like changing tests, but I think there was a bug before which 
were never hit in our tests but here.

Before
```
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=130 Name=decltype L=42 PPK=2 
FakeLParens=5/ FakeRParens=0 II=0xf136c40 Text='decltype'
 M=0 C=0 T=Unknown S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=43 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=LambdaLSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=44 PPK=2 
FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=45 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=46 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=47 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='->'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=54 PPK=2 
FakeLParens= FakeRParens=0 II=0xf130c60 Text='std'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=coloncolon L=56 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=540 Name=identifier L=65 PPK=2 
FakeLParens= FakeRParens=0 II=0xf130c88 Text='true_type'
 M=0 C=0 T=LambdaLBrace S=1 F=0 B=0 BK=1 P=43 Name=l_brace L=67 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='{'
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='}'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=81 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=82 PPK=2 FakeLParens= 
FakeRParens=1 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=83 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=85 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=TrailingAnnotation S=0 F=0 B=0 BK=0 P=520 Name=identifier L=90 PPK=2 
FakeLParens= FakeRParens=0 II=0xf130ca8 Text='value'
 M=0 C=1 T=BinaryOperator S=1 F=0 B=0 BK=0 P=25 Name=ampamp L=93 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='&&'
```
after
```
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=130 Name=decltype L=42 PPK=2 
FakeLParens=5/ FakeRParens=0 II=0xee66c40 Text='decltype'
 M=0 C=0 T=TypeDeclarationParen S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=43 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=LambdaLSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=44 PPK=2 
FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=45 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=46 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=47 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='->'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=54 PPK=2 
FakeLParens= FakeRParens=0 II=0xee60c60 Text='std'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=coloncolon L=56 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=540 Name=identifier L=65 PPK=2 
FakeLParens= FakeRParens=0 II=0xee60c88 Text='true_type'
 M=0 C=0 T=LambdaLBrace S=1 F=0 B=0 BK=1 P=43 Name=l_brace L=67 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='{'
 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='}'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=81 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=82 PPK=2 FakeLParens= 
FakeRParens=1 II=0x0 Text=')'
 M=0 C=0 T=TypeDeclarationParen S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=83 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=85 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=520 Name=identifier L=90 PPK=2 
FakeLParens= FakeRParens=0 II=0xee60ca8 Text='value'
 M=0 C=0 T=BinaryOperator S=1

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

2022-03-13 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, curdeius, MyDeveloperDay.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/54350

As driveby rename `Left` to `OpeningParen`, this gives a better overview what 
it is, and we have  a loop in the function, where `Left` is not adjusted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121550

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -331,6 +331,14 @@
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[29], tok::kw_requires,
TT_RequiresClauseInARequiresExpression);
+
+  // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+  Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
+"requires true; };");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_RequiresExpressionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, RequiresDoesNotChangeParsingOfTheRest) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23891,9 +23891,9 @@
   verifyFormat(
   "template  concept C = decltype([]() -> std::true_type {\n"
   "return {};\n"
-  "  }())::value\n"
-  "  && requires(T t) { t.bar(); } &&\n"
-  "  sizeof(T) <= 8;",
+  "  }())::value &&\n"
+  "  requires(T t) { t.bar(); } && "
+  "sizeof(T) <= 8;",
   Style);
 
   verifyFormat("template  concept Semiregular =\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -211,32 +211,34 @@
   bool parseParens(bool LookForDecls = false) {
 if (!CurrentToken)
   return false;
-FormatToken *Left = CurrentToken->Previous;
-assert(Left && "Unknown previous token");
-FormatToken *PrevNonComment = Left->getPreviousNonComment();
-Left->ParentBracket = Contexts.back().ContextKind;
+assert(CurrentToken->Previous && "Unknown previous token");
+FormatToken  = *CurrentToken->Previous;
+assert(OpeningParen.is(tok::l_paren));
+FormatToken *PrevNonComment = OpeningParen.getPreviousNonComment();
+OpeningParen.ParentBracket = Contexts.back().ContextKind;
 ScopedContextCreator ContextCreator(*this, tok::l_paren, 1);
 
 // FIXME: This is a bit of a hack. Do better.
 Contexts.back().ColonIsForRangeExpr =
 Contexts.size() == 2 && Contexts[0].ColonIsForRangeExpr;
 
-if (Left->Previous && Left->Previous->is(TT_UntouchableMacroFunc)) {
-  Left->Finalized = true;
+if (OpeningParen.Previous &&
+OpeningParen.Previous->is(TT_UntouchableMacroFunc)) {
+  OpeningParen.Finalized = true;
   return parseUntouchableParens();
 }
 
 bool StartsObjCMethodExpr = false;
-if (FormatToken *MaybeSel = Left->Previous) {
+if (FormatToken *MaybeSel = OpeningParen.Previous) {
   // @selector( starts a selector.
   if (MaybeSel->isObjCAtKeyword(tok::objc_selector) && MaybeSel->Previous &&
   MaybeSel->Previous->is(tok::at))
 StartsObjCMethodExpr = true;
 }
 
-if (Left->is(TT_OverloadedOperatorLParen)) {
+if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
   // Find the previous kw_operator token.
-  FormatToken *Prev = Left;
+  FormatToken *Prev = 
   while (!Prev->is(tok::kw_operator)) {
 Prev = Prev->Previous;
 assert(Prev && "Expect a kw_operator prior to the OperatorLParen!");
@@ -255,54 +257,58 @@
   // type X = (...);
   // export type X = (...);
   Contexts.back().IsExpression = false;
-} else if (Left->Previous &&
-   (Left->Previous->isOneOf(tok::kw_static_assert, tok::kw_while,
-tok::l_paren, tok::comma) ||
-Left->Previous->isIf() ||
-Left->Previous->is(TT_BinaryOperator))) {
+} else if (OpeningParen.Previous &&
+

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

[PATCH] D121550: [clang-format] Fix crash on invalid requires expression

10 matches

Site Navigation

Mail list logo

Footer information