[llvm] [z/OS][GOFF] Implement support for writing ESD + TXT records by the GOFFObjectWriter (PR #85851)

[Ulrich Weigand via llvm-commits]

https://github.com/uweigand commented:

I'll go through in more detail later, but a couple of general comments first:
- Some of the information provided in the commit message here should really go as comments into the source as well.
- This PR seems to have "policy" and "implementation" aspects very closely intertwined.  I think from a general maintainability perspective it would be much preferable to implement a clearer separation of concerns.  For example, I don't think the section writer should ever look at the section name to decide what to do - instead, whoever creates the section should use generic mechanism like attributes to describe the requirements this particular section has, and then the section writer would just follow those instructions generically.
- There are a number of GOFF-specific symbol attributes implemented by this patch where I'm wondering if we couldn't use existing generic attributes instead.  In particular, the "setExecutable" attribute seems to track the same type of information tracked by the "MCSA_ELF_Type*" symbol attributes.  If we could hook into this, we'd benefit from common code already setting these attributes as needed.
- The patch seems incomplete/partial in certain aspects.  E.g. there are various flags that the code attempts to implement, but there is no place where they are ever set.  I think it would be better to commit an initial version of this patch *without* those flags, and then add them back in follow-on patches, together with the places where they're actually used and test cases.
- The whole GlobalAlias handling seems to duplicate/conflict with this PR https://github.com/llvm/llvm-project/pull/84829 - in either case, I'm still not convinced this is the best approach.  Again, it would probably best to leave all that out of this PR and add it later separately.

https://github.com/llvm/llvm-project/pull/85851