[PATCH] D20337: [MC] Support symbolic expressions in assembly directives

[Rafael Espíndola via llvm-commits]

On May 23, 2016 10:54 PM, "Nirav Dave" <niravd at google.com> wrote:
>
> niravd added inline comments.
>
> ================
> Comment at: include/llvm/MC/MCStreamer.h:591
> @@ +590,3 @@
> +  /// \param Loc - The location of the expression for error reporting.
> +  virtual void emitSpace(const MCExpr *Size, uint64_t FillValue,
> +                         SMLoc Loc = SMLoc());
> ----------------
> Please rename to match casing of other Emit functions. (EmitSpace).

No. Use the new stile or we will never be done moving to it. If you want
consistency, rename the existing ones and then rebase.

> ================
> Comment at: include/llvm/MC/MCStreamer.h:602
> @@ +601,3 @@
> +  /// \param Loc - The location of the expression for error reporting.
> +  virtual void emitRepeatedValue(const MCExpr *NumValues, int64_t Size,
> +                                 int64_t Expr, SMLoc Loc = SMLoc());
> ----------------
> Why isn't this named EmitFill? It's of a distinct type from the other
calls and is only for dealing with Fill directives and the distinction
seems more confusing than multiple variants of EmitFill.
>
> ================
> Comment at: include/llvm/MC/MCStreamer.h:605
> @@ +604,3 @@
> +
> +  void emitRepeatedValueImpl(int64_t Repeat, int64_t Size, int64_t Value,
> +                             SMLoc Loc = SMLoc());
> ----------------
> Save as above.
>
> ================
> Comment at: lib/MC/MCAsmStreamer.cpp:828
> @@ +827,3 @@
> +
> +  OS << "\t.space\t";
> +  NumBytes->print(OS, MAI);
> ----------------
> Since we cannot guarantee a lack of errors at this point we should emit
line directives before and after this so that we preserve the error message
location with respect to the original file.
>
> ================
> Comment at: lib/MC/MCParser/AsmParser.cpp:2749
> @@ -2747,2 +2748,3 @@
>      Lex();
>      if (parseAbsoluteExpression(Val))
> +      return false;
> ----------------
> Please decompose this into parseExpression and evaluateAsAbsolute to
distinguish expression structure errors (should throw an error here) from
value errors which we may defer.
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D20337
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160524/016ad9fb/attachment.html>