[llvm] [Object][ELF] Outline section-content validation errors (PR #202758)

[David Zbarsky via llvm-commits]

dzbarsky wrote:

Thanks for taking a look!

> My first immediate thought is: how are you going to prevent this regressing?

Great question. My instinct would be to start collecting binary size metrics in CI so we can at least warn on PRs if they inadvertently regress these metrics, but of course that would be a bigger project and require a bit of cultural buy-in for binary size being something we care about.

> It's not obviously better code (without the context of the size info you mention in the PR description) - indeed, I'd argue it at least partially makes the code less readable, because it moves the details of the error messages away from the check site (though I acknowledge that this is a subjective thing). If I came to this code without the context of this PR, I'd want to inline the error reporting, because it doesn't make sense out-of-line: we don't typically have simple helper methods out-of-line if they only have one call site.

Indeed. The core problem IMO is that c++ templates make it very easy to incur inadvertent codesize expansion. Perhaps there are other refactorings we could do to this code to get a similar win that would feel more maintainable, though I didn't see one offhand.

> I'm assuming that the reasoning for the size decrease is because prior to your change, each of these bits of code would be duplicated, because you'd end up with one instance per template instantiation. Is that correct?

Yes.

> The size gains you've discussed are also tiny. Instinctively, my feeling is that this isn't worth the reduction in readability, as a result.

This particular change of 100KB is fairly small. [I had an agent look across the codebase and there are many similar opportunities](https://github.com/llvm/llvm-project/issues/202616); in aggregate the potential win is fairly large (30MB+). The nature of this is that there are some targeted changes that save 500KB+ and a slew of smaller changes that add up. (Many of the PRs linked there are still in draft as they need cleanup, some are likely overly complex and not worth it, etc. But hopefully the broader point makes sense!)

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