[PATCH] [lld] Proposal and example for changing Atom attributes.

[Rui Ueyama]

================
Comment at: include/lld/Core/DefinedAtom.h:233
@@ +232,3 @@
+  /// \brief Alter the visibility of this atom.
+  virtual void setScope(Scope);
+
----------------
I assume we will add more setXX member functions (e.g. setInterposable) when we need it. I don't like to add them now but want to make sure that we are on the same page.

================
Comment at: include/lld/Core/LinkingContext.h:230
@@ +229,3 @@
+  /// as adjusting the atom's scope.
+  virtual void notifySymbolTableAdd(const Atom *atom) const {
+  }
----------------
This function signature made me a bit sad because it's intended to mutate a given atom despite it takes const Atom *. We may probably want to make the atom graph mutable in the first place, because it's no longer actually const because of the addition of this function, so that we don't have to sprinkle const_casts in many places. I don't like to see that change in this CL but it may need to be considered later.

================
Comment at: include/lld/Core/LinkingContext.h:239
@@ +238,3 @@
+                                         const Atom *newAtom,
+                                         bool &useNew) const {
+  }
----------------
I like this function. I'll use it for PE/COFF as well -- when an undefined atom specified by "--undefined" (or "/include" on Windows) is coalesced with a defined atom, I want to copy that atom's deadStrip attribute to a new atom.

================
Comment at: lib/Core/DefinedAtom.cpp:87
@@ +86,3 @@
+void DefinedAtom::setScope(Scope s) {
+  assert(s == scope() && "setScope() must be overridden to alter scope");
+}
----------------
Does this have to be compared with the current value? I don't think we want to call setScope() if it's not really implemented. It seems to me that we can just put llvm_unreachable here.

================
Comment at: lib/ReaderWriter/MachO/MachOLinkingContext.cpp:585
@@ +584,3 @@
+
+  if (currentlyGlobal != whiteListed) {
+    if (currentlyGlobal) {
----------------
You can early return here if you invert the condition inside if.

================
Comment at: lib/ReaderWriter/MachO/MachOLinkingContext.cpp:602
@@ +601,3 @@
+  if (currentlyGlobal == blackListed) {
+    if (currentlyGlobal) {
+      // Reduce scope of symbols in unexport list down to "hidden".
----------------
if (currentlyGlobal && currentlyGlobal == blacklisted)?

http://reviews.llvm.org/D4965