[PATCH] D118476: [demangler] Fix new/delete demangling

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: libcxxabi/src/demangle/ItaniumDemangle.h:4821
+      NodeArray Inits = popTrailingNodeArray(InitsBegin);
+      return make<NewExpr>(ExprList, Ty, Inits, Global, IsArray);
+    }
----------------
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'.


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

https://reviews.llvm.org/D118476