[PATCH] MC asm parser: allow @'s in symbol names in MS assembly

Hans Wennborg hans at chromium.org
Fri Oct 18 12:10:59 PDT 2013


  > Do we already have a testcase showing that we reject foo at bar on ELF? If not, it would be a good idea to add one.

  We don't. I'll add one.


================
Comment at: lib/MC/MCParser/AsmLexer.cpp:143
@@ -142,3 +142,3 @@
 static bool IsIdentifierChar(char c) {
-  return isalnum(c) || c == '_' || c == '$' || c == '.' || c == '@';
+  return isalnum(c) || c == '_' || c == '$' || c == '.' || c == '@' || c == '?';
 }
----------------
Rafael Ávila de Espíndola wrote:
> adding support for ? in a name can be an independent change, no?
> 
> btw, on linux gas rejects
> 
> .globl foo?bar
> 
> Should be reject too? I am tempted to just accept it everywhere as an extension, since it doesn't look ambiguous (unlike the @) 
Right, this would be an extension, and like you say I think it would be harmless - I don't think the lexer actually accepts in anywhere else.

gas doesn't accept this, but these symbols should not be showing up in asm the we send to gas.

I could do this in an independent change, but I think it ties in nicely with the test I'm adding here. Happy to split it out if you think it's important though.


http://llvm-reviews.chandlerc.com/D1978



More information about the llvm-commits mailing list