[PATCH] D118476: [demangler] Fix new/delete demangling
Nathan Sidwell via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 9 08:40:20 PST 2022
urnathan added inline comments.
================
Comment at: libcxxabi/src/demangle/ItaniumDemangle.h:4821
+ NodeArray Inits = popTrailingNodeArray(InitsBegin);
+ return make<NewExpr>(ExprList, Ty, Inits, Global, IsArray);
+ }
----------------
ChuanqiXu wrote:
> urnathan wrote:
> > urnathan wrote:
> > > ChuanqiXu wrote:
> > > > In case `HaveInits` is false and `consumeIf('E')` return true, the original behavior would be `return make<NewExpr>(ExprList, Ty, NodeArray(), Global, IsArray);`. Is the current behavior expected?
> > > You're misreading the old code -- there's a negation on the non-looping consume( 'E'). There's always an 'E', it's just that without 'pi' that 'E'must be immediate (there are no expressions).
> > >
> > > Here's the grammar -- you always reach an 'E'
> > >
> > > // [gs] nw <expression>* _ <type> E # new (expr-list) type
> > > // [gs] nw <expression>* _ <type> <initializer> # new (expr-list) type (init)
> > > // [gs] na <expression>* _ <type> E # new[] (expr-list) type
> > > // [gs] na <expression>* _ <type> <initializer> # new[] (expr-list) type (init)
> > > // <initializer> ::= pi <expression>* E # parenthesized initialization
> > >
> > > (Not sure why the 'pi' initializer introducer is needed, probably history.)
> > >
> > > OLD:
> > > if ('pi') { while not 'E' {read expr; } build newexpr (exprs)
> > > else if not 'E'; fail
> > > else build newexpr ();
> > >
> > > NEW:
> > > haveinits = 'pi';
> > > while not 'E' { if !haveinits; fail, else read expr }
> > > build newexpr (exprs)
> > >
> > oh, of course 'pi' is needed as 'new int' and 'new int ()' are different. Not sure why 'new int {}' is not representable, perhaps no one's got there yet?
> Yeah, the point here is if 'pi' is not true, then there must be an immediate 'E'. Then the code makes sense. It is a little surprising to me that the parser of demangler wouldn't handle the wrong case. Maybe the basic assumption is that the input is valid.
>
> For the case of 'new int {}', I guess it is represented as there are multiple <expression> between pi and 'E'.
I'm not sure I follow about 'wouldn't handle the wrong case'. The parser should definitely not assume the input is valid. What invalid input have I missed?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118476/new/
https://reviews.llvm.org/D118476
More information about the llvm-commits
mailing list