[PATCH] [inline-asm] Fix scope of assembly macros
Reid Kleckner
rnk at google.com
Tue Dec 2 11:31:48 PST 2014
I guess we need this if we want gcc compatibility. Users shouldn't have to repeat macros in every inline asm blob, which gcc will then reject, forcing them to use ifdefs. =P
================
Comment at: lib/MC/MCParser/AsmParser.cpp:55-56
@@ +54,4 @@
+/// string is independent from its creator.
+class MCAsmMacroArgToken
+{
+ /// Storage for token value.
----------------
Please don't use Allman bracing, put the following open brace on the same line for ifs, fors, tag types, etc.
================
Comment at: lib/MC/MCParser/AsmParser.cpp:529
@@ -488,3 +528,3 @@
AsmParser::AsmParser(SourceMgr &_SM, MCContext &_Ctx, MCStreamer &_Out,
const MCAsmInfo &_MAI)
----------------
This constructor will be removed once clang is updated, right?
================
Comment at: lib/MC/MCParser/AsmParser.cpp:546-552
@@ +545,9 @@
+{
+ CurBuffer = SrcMgr.getMainFileID();
+ MacrosEnabledFlag = true;
+ HadError = false;
+ CppHashLineNumber = 0;
+ AssemblerDialect = ~0U;
+ IsDarwin = false;
+ ParsingInlineAsm = false;
+
----------------
Please leave these in initializer lists, even at the cost of duplication. The duplication will be gone once the alternate ctor is deleted.
================
Comment at: lib/MC/MCParser/AsmParser.cpp:1918
@@ -1858,1 +1917,3 @@
+MCAsmMacroArgToken::MCAsmMacroArgToken(const AsmToken &_Token) {
+ Value = _Token.getString();
----------------
Underscores in variable names aren't really used in LLVM. Can you name this parameter something like TokenToCopy or CopiedToken?
================
Comment at: lib/MC/MCParser/AsmParser.cpp:4803
@@ +4802,3 @@
+
+MCAsmContext::MCAsmContext()
+{
----------------
More Allman
http://reviews.llvm.org/D6383
More information about the llvm-commits
mailing list