[PATCH] D65273: [llvm-objcopy] - Reimplement strip-dwo-groups.test to stop using the precompiled object.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/strip-dwo-groups.test:5
 
-// Source code of groups.o:
-//
-// template <class T>
-// struct S {
-//    static constexpr T X = T(1);
-//    T getX() { return X; }
-// };
-// void f() {
-//  S<int> A;
-//  S<double> B;
-//  int a = A.getX();
-//  int b = B.getX();
-// }
-//
-// clang -g -gsplit-dwarf -std=c++11 -c groups.cpp -o groups.o
+## `llvm-objcopy --strip-dwo` strips out dwo sections, as a result, the index of 
+## the symbol table, the indices of the symbols and the indices of the sections
----------------
MaskRay wrote:
> The comment may be too verbose. Suggested wording (please improve as you rewrite :) ):
> 
> After the removal of .dwo sections, the indices of sections which go after them will change. Check that the sh_link and sh_info fields of .group and their contents (indices of member sections) are updated.
Your version looks OK to me.

But I do not feel that "too verbose" is a problem,
I like to read the description of the problem and the details.
Unfortunately often svn/git commit comments are the only source.
"too verbose" is in my understanding can be used for the code comments,
but while we are in the test cases area, If you do not mind, I would keep it verbose.


================
Comment at: test/tools/llvm-objcopy/ELF/strip-dwo-groups.test:69
+    Info:  .text.group2
+    Relocations:
+  - Name: .debug_after.dwo
----------------
MaskRay wrote:
> Delete the empty `Relocations:`
Done (I thought this is a mandatory field, but it is not).


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

https://reviews.llvm.org/D65273