[PATCH] D60306: Fix -emit-reloc against local symbols.

Tue Apr 16 07:47:58 PDT 2019

MaskRay added inline comments.

================
Comment at: lld/trunk/test/ELF/emit-relocs-mergeable2.s:6
+
+# CHECK: 0000000000201004  000000010000000b R_X86_64_32S 0000000000200120 .Lfoo + 8
+
----------------
grimar wrote:
> ruiu wrote:
> > grimar wrote:
> > > grimar wrote:
> > > > I would suggest adding the header for these values and a short description of what this patch does.
> > > > (I know we are missing the descriptions often, but ideally, I think we might want to add a description for each of
> > > > our test. At least that is something I have in my TODO list for LLD and it is really useful sometimes).
> > > what this patch does -> what this test checks
> > Honestly, unlike code, I don't think we need to explain the test in details. If you will need to change this test in the future, for example, it is very likely that you are changing the code and already understand what is expected.
> I disagree here. It is sometimes hard to realize what the test does. I do not see any reason why we can't add a short description for each.
> Perhaps the problem is that I still do not understand what this test do? And then this is what I am talking about: each test should have a clear description.
I agree with George that a short description will be helpful.

1) help understand the code
2) convenience for people who make layout changes and have to adjust many tests

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60306/new/

https://reviews.llvm.org/D60306