[PATCH] D13659: Implement .reloc (constant offset only) with support for R_MIPS_NONE and R_MIPS_32.

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 26 16:10:23 PDT 2015


majnemer added inline comments.

================
Comment at: include/llvm/MC/MCStreamer.h:692
@@ +691,3 @@
+                                  const MCExpr &Expr, SMLoc Loc = SMLoc()) {
+    return false;
+  }
----------------
dsanders wrote:
> majnemer wrote:
> > Shouldn't this return `true` because nothing was emitted?
> I think `false` makes the most sense. Returning true would cause all targets to silently accept and ignore any relocation name. By returning false, we emit an error message about the unknown/unsupported name.
Your comment says:
>   Returns true if the relocation could not be emitted because Name is not known.

All relocations cannot be emitted by this implementation because `Name` will never be known, hence the function should return `true`.

If you'd be happier with it returning false, that's fine but please change the comment to be consistent.  Also, this case needs a test.


http://reviews.llvm.org/D13659





More information about the llvm-commits mailing list