[llvm-commits] [llvm] r125595 - in /llvm/trunk: lib/MC/MCParser/AsmParser.cpp test/MC/AsmParser/exprs.s test/MC/AsmParser/paren.s

[Rafael Ávila de Espíndola]

> Joerg and others,
> 
> It looks like there are a couple of misunderstandings going on here.
> I really appreciate the work you've been doing and I agree that the
> patch to add [] is a good feature for the X86 backend.
> Unfortunately, Jim is right that it does break perfectly legal ARM
> code (and Jim should add a testcase for this code that is getting
> broken!).
> 
> I don't think that this is a matter of "what is right or wrong" or
> "who" is right.  The general policy we follow in LLVM (documented
> here: http://llvm.org/docs/DeveloperPolicy.html#quality) is that we
> don't want to regress currently working stuff.  Jim's coming at this
> from the perspective that your patch broke something on ARM and
> therefore introduced a regression.  I think it's lousy that there
> isn't a regression test for this, but these things do happen.
> 
> In any case, instead of focusing on who is right or wrong, it is more
> productive to focus on how to make progress here.  I agree with Jim
> that reverting the patch for the time being is the right thing to do.
> It would be great if you could resubmit the patch, adding a target
> hook at the the ARM assembler can override to disable [] parsing.  I
> don't think that this would be a major extension to the patch and
> would be happy to help with it.
> 
> Just remember, we're all friends here.  :-)

Just a nit, it looks like it is a Mach-O X ELF, not X86 x ARM.

Just to be sure, would this be a desirable feature for X86 Darwin?

Now that I know that existing assemblers for ARM Mach-O don't support
this, I agree it is not a good idea to be the first one to do so.

Since this is in the expression parser, we probably need a
HasFunnySquareBracketExpr flag or something, but yes, it should not be a
big change on top of the previous patch.

> -Chris

Cheers,
Rafael