[PATCH] D82157: Fix crash on `user defined literals`

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:737
+          Builder.findToken(TokLoc)->text(Context.getSourceManager());
+      auto Literal = NumericLiteralParser{TokSpelling,
+                                          TokLoc,
----------------
Please use parentheses to call the constructor. Braces are correct, but not idiomatic in LLVM.


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:754
+                     new (allocator()) syntax::UserDefinedLiteralExpression(
+                         getUserDefinedLiteralKind(S)),
+                     S);
----------------
Please allocate an instance of the correct derived type. It is undefined behavior to downcast an object allocated as `UserDefinedLiteralExpression` to any of its subtypes.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1197
+
+unsigned operator "" _r(const char*); // raw-literal operator
+
----------------
No need to explain the language.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1200
+template <char...>
+unsigned operator "" _t();            // numeric literal operator template
+
----------------
No need to explain the language.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1210
+  12_t;  // call: operator<'1', '2'> "" _x() | kind: integer
+  1.2_t; // call: operator<'1', '2'> "" _x() | kind: float
 }
----------------
call -> calls? (as in, "this expression calls ...")


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1333
+      R"cpp(
+typedef decltype(sizeof(void *)) size_t;
+unsigned operator "" _s(const char*, size_t);
----------------
I don't understand why this test is separate from the previous one -- why not merge them all into one, or split all of them into one call per test?


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1336
+void test() {
+  "12"_s;// call: operator "" _s("12")       | kind: string
+}
----------------
ditto, "calls"

Also please add a space before "//".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82157