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

[Shankar Kalpathi Easwaran]

================
Comment at: include/lld/Core/LinkingContext.h:230
@@ +229,3 @@
+  /// as adjusting the atom's scope.
+  virtual void notifySymbolTableAdd(const Atom *atom) const {
+  }
----------------
kledzik at apple.com wrote:
> Rui Ueyama wrote:
> > 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.
> Yes. For now the graph is 99.9% const, so a few const_cast<> keeps it closer to what we are trying to model.  But if we have Passes that need to modify atoms, or lots of places need start modifying atoms, it will probably make sense to remove the const in the zillion places.
Various flavors can go over the atoms in pass and set the appropriate attributes right, Am I missing something ? Same with the below function as well.

================
Comment at: lib/Core/DefinedAtom.cpp:86
@@ -83,1 +85,3 @@
 
+void DefinedAtom::setScope(Scope s) {
+  assert(s == scope() && "setScope() must be overridden to alter scope");
----------------
I think visibility and scope are being mixed. While scope needs to be a property of the atom, I am not convinced visibility has to be part of the Atom has well, as visibility is mostly user driven.

================
Comment at: lib/ReaderWriter/MachO/MachOLinkingContext.cpp:588
@@ +587,3 @@
+      // Reduce scope of symbols not in export list down to "hidden".
+      const_cast<DefinedAtom *>(atom)->setScope(Atom::scopeLinkageUnit);
+    } else {
----------------
Arent we mixing visibility and scope here ? I think its better to add visibility as a separate attribute.

================
Comment at: lib/ReaderWriter/Native/ReaderNative.cpp:888-894
@@ +887,9 @@
+
+inline void NativeDefinedAtomV1::setScope(DefinedAtom::Scope s) {
+  // Do nothing is scope is already requested value.
+  if (scope() == s)
+    return;
+  // Changing scope is rare.  Store changes in side table.
+  _file->_atomAttributeOverrides[this].scope = s;
+}
+
----------------
Is this needed anytime ? What kind of usecase is this ?

http://reviews.llvm.org/D4965