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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits