[PATCH] D37413: [X86][MS-InlineAsm] Extended support for variables / identifiers on memory / immediate expressions

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: lib/Sema/SemaStmtAsm.cpp:617
+    return;
+  } else if (Res->isRValue()) {
+    bool Enum = isa<clang::EnumType>(T) && Res->EvaluateAsRValue(Eval, Context);
----------------
RKSimon wrote:
> (style) Split these instead of an if-elseif chain
LLVM suggests early returns: https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code

I don't particularly care here, up to you. I personally like to abuse the ability of C++ to return void to write things like this and skip braces, but maybe *I'm* too clever for my own good:
  if (T->isFunctionType() || T->isDependentType())
    return Info.setLabel(Res);
  if (Res->isRValue()) {
    if (isa<clang::EnumType>(T) && Res->EvaluateAsRValue(Eval, Context))
      return Info.setEnum(Eval.Val.getInt().getSExtValue());
    return Info.setLabel(Res);
  }


================
Comment at: lib/Sema/SemaStmtAsm.cpp:620
+    bool Enum = isa<clang::EnumType>(T) && Res->EvaluateAsRValue(Eval, Context);
+    Enum ? Info.setEnum(Eval.Val.getInt().getSExtValue()) : Info.setLabel(Res);
+    return;
----------------
This conditional expression is a bit too cute, I'd rather use a regular if.


Repository:
  rL LLVM

https://reviews.llvm.org/D37413