[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