sammccall accepted this revision.
sammccall added inline comments.
================
Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:85
+ static constexpr unsigned SizeBits = 4;
+ static_assert(SizeBits + SymbolBits <= 16,
+ "Must be able to store symbol ID + size efficiently");
----------------
hokein wrote:
> sammccall wrote:
> > We're being quite fiddly with layout but actually pretty wasteful.
> >
> > wc says 652 with 1323 total RHS elements on the RHS - average ~2.
> > So we could aim for 6 bytes per rule = 4KB but we're using 34 bytes per
> > rule = 22KB.
> >
> > Can't see a nice concrete way to achieve that though.
> Yes :(
> I'd keep it as-is (and decrease the MaxElements to 9). One idea is to use a
> flat array as a center storage of all sequences, then Rule class just need an
> index, and size. But there will be more data (annotations, hooks) in this
> class, we could figure it out later.
> Yes :(
> I'd keep it as-is (and decrease the MaxElements to 9).
SGTM
> One idea is to use a flat array as a center storage of all sequences, then
> Rule class just need an index, and size.
This makes APIs awkward though: Rule.seq() needs extra params to build an
ArrayRef, unless you want to carry around a pointer to the big array (which
gives back most of the size savings)
================
Comment at: clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp:140
+ for (llvm::StringRef Line : llvm::split(Lines, '\n')) {
+ Line = Line.trim();
+ // Strip anything coming after the '#' (comment).
----------------
move this after the take_while, so that ` # this is a comment` is still
skipped?
================
Comment at: clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp:153
+
+ bool ParseLine(llvm::StringRef Line, RuleSpec &Out) {
+ auto Parts = Line.split(":=");
----------------
ParseLine -> parseLine
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114790/new/
https://reviews.llvm.org/D114790
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
