[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:4813-4814
+      while (!consumeIf('E')) {
+        if (!HaveInits)
+          return nullptr;
+        Node *Init = getDerived().parseExpr();
----------------
ChuanqiXu wrote:
> `HaveInits` is loop invariant,  could we hoist the check?
no, see below


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



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

https://reviews.llvm.org/D118476