[llvm-commits] [PATCH] [lld] Factor File::addAtom out to MutableFile::addAtom.

[Sean Silva]

  > Is it worth it to introduce a new class just to get rid of some llvm_unreachables?
  >
  > The mutability was already enforced via const.  That is, only a non-const File could have its addAtoms() method called.

  IMO yes. It is a violation of the Liskov substitution principle otherwise (not that we need to dogmatically follow LSP, but the LSP aligns closely with intuition anyway).

  Additionally, ensuring that all non-mutable atoms don't inherit from MutableAtom requires O(# non-mutable atom classes) effort, and any anomalies can be seen easily from a doxygen inheritance graph. If we only rely on `const` then we need to check O(# places where an Atom type is used) to ensure it, and we have no easy centralized way to audit those places. Also, experience shows that programmers do cast away const in practice; on the other hand, no programmer working on LLVM would be so stupid as to do a cross-cast into another hierarchy, since that is "obviously" undefined behavior.

http://llvm-reviews.chandlerc.com/D274