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

[Daniel Sanders via llvm-commits]

dsanders added inline comments.

================
Comment at: include/llvm/MC/MCStreamer.h:692
@@ +691,3 @@
+                                  const MCExpr &Expr, SMLoc Loc = SMLoc()) {
+    return false;
+  }
----------------
majnemer wrote:
> 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.
That'll teach me to use true to indicate failure. Sorry, you're absolutely right that I'm returning success instead of failure in this implementation. I'll fix this.


http://reviews.llvm.org/D13659