[PATCH] D86574: [lld-macho] Emit the right header flags for weak bindings/symbols

Jez Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 26 23:07:07 PDT 2020


int3 added inline comments.


================
Comment at: lld/MachO/SyntheticSections.cpp:429
       trieBuilder.addSymbol(*defined);
+      hasWeakSymbol |= sym->isWeakDef();
+    }
----------------
smeenai wrote:
> This is ridiculously nitty on my part, but if you write this instead, it'll get short-circuited.
I know... I just hate that C++ doesn't support `||=`. I'll change it :p 


================
Comment at: lld/test/MachO/weak-header-flags.s:16
+# WEAK-BINDS-ONLY:        MH_BINDS_TO_WEAK
+# WEAK-BINDS-ONLY-NOT:    MH_WEAK_DEFINES
+
----------------
smeenai wrote:
> Nit: I'd prefer an `--implicit-check-not MH_WEAK_DEFINES` in the FileCheck command instead, to catch it both before or after the `MH_BINDS_TO_WEAK` (or you could also add the `NOT` check before as well). I recognize that readobj will probably print them in the order you're checking (and the above test would fail if that order changed anyway), so this is a purely hypothetical concern :)
I dislike how `--implicit-check-not` is placed far away from the other check statements, but putting it before and after the positive check here sounds good


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86574



More information about the llvm-commits mailing list