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

[Nathan Sidwell via Phabricator via llvm-commits]

urnathan marked 2 inline comments as done.
urnathan 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:
> 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?


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

https://reviews.llvm.org/D118476