[PATCH] D124792: [ifs] Add --strip-size flag

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I'd like to see an llvm-ifs Command Guide document wtitten sooner rather than later, since all llvm tools should have one. Please look at doing this soon.



================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:222
+      if (Size == std::numeric_limits<uint64_t>::max())
+        Size = 1;
+      DynSym.Content.add(DynStr.Content.getOffset(Sym.Name), Size, Bind,
----------------
Why is `Size` set to 1 and not 0, which would seem the more natural value?


================
Comment at: llvm/test/tools/llvm-ifs/strip-size.test:7
+
+## Check that the size when emitting to elf defaults to 1
+# RUN: llvm-ifs %s --output-elf - --strip-size | llvm-ifs - --output-ifs - | \
----------------



================
Comment at: llvm/tools/llvm-ifs/llvm-ifs.cpp:441
+    for (auto &Sym : Stub.Symbols)
+      Sym.Size = std::numeric_limits<uint64_t>::max();
+
----------------
Could you make `Sym.Size` `Optional` instead? That would seem like a cleaner interface to me.


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

https://reviews.llvm.org/D124792