erik.pilkington added inline comments.
Comment at: src/cxa_demangle.cpp:1575-1577
- sub_type names;
- template_param_type subs;
- Vector<template_param_type> template_param;
> - Why not rename `names` as well?
> - Please rename these in a separate prep commit to minimize noise on the
> final patch.
Comment at: src/cxa_demangle.cpp:1604
+ assert(N < PackIndices.size());
+ Node **Begin = Subs.begin() + PackIndices[N];
+ Node **End = (N + 1 == PackIndices.size())
> Should there be some assertion on `Subs.size()` here? Perhaps, if so, this
> should be `&Subs[PackIndices[N]]`, so that `Subs.operator()` handles it for
> you. Same below.
We can't use operator here because if all we have is an empty parameter pack
then `&Subs[PackIndices[N]]` needs to point the the end() iterator, which is
out of bounds. The new patch adds an assert though.
Comment at: src/cxa_demangle.cpp:1621-1631
+ // Name stack, this is used by the parser to hold temporary names that were
+ // parsed. The parser colapses multiple names into new nodes to construct
+ // the AST. Once the parser is finished, names.size() == 1.
+ PODSmallVector<Node*, 32> names;
+ // Substitution table. Itanium supports name substitutions as a means of
+ // compression. The string "S42_" refers to the 42nd entry in this table.
> How much have you thought about these sizes (32, 32, and 4)? Any evidence to
> back up these specific choices?
Yep, 32 is enough for the Subs table/names to never overflow when demangling
the symbols in LLVM, and 4 seldom overflows TemplateParams. TemplateParams is
temporary held on the stack in parse_template_args(), so I wanted it to be
cfe-commits mailing list