[llvm-commits] [llvm] r156714 - in /llvm/trunk: lib/MC/MCParser/AsmParser.cpp test/MC/AsmParser/macro-rept-err1.s test/MC/A

Rafael Espíndola rafael.espindola at gmail.com
Mon May 14 16:12:27 PDT 2012


> Hi Rafael,
>
> can you please look into my version i posted earlier today on this list? i think my
> solution is better as it implements more directives and does much less copy-pasting
> of code.

Sure. Sorry for bad timing on my part. I like the general approach of
handling the new directives as special macros.

One nit, don't leave comment code in:

+//  if (getLexer().isNot(AsmToken::EndOfStatement))
+//    return TokError("unexpected token in '.macro' directive");

Can you split the new predicates into a patch? I am much more
comfortable reviewing that than the changes to .macro.

Can you add testcases showing that the nesting works? Nesting with
.macro in particular.

+  if (Name.size())
+    OS << ".endmacro\n";

There is only one caller now that wants to add .endmacro, no? It is
probably better to move this code there.

> thanks,
>
>  PaX Team
>

Cheers,
Rafael




More information about the llvm-commits mailing list