[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