[llvm] [AsmPrinter] Find a DIE's unit by its owner, not by its tag (PR #219660)

[Scott Linder via llvm-commits]

slinder1 wrote:

@arcivanov I am sorry if I am overly blunt here, but if I take your interactions in good-faith I'm left to assume you still haven't really understood the underlying issues with your submissions, which will be a problem as there are a dozen or so more left to be reviewed, so I want to be as clear as possible.

I think I speak for the whole LLVM community when I say I'm glad you decided to contribute to the project, and I appreciate the work you put in! However, there is no middle-ground where respect for maintainers is concerned.

> @JDevlieghere
> 
> 1. Every single word of the Summary is _my own word_ - I kept restructuring, rewording and compressing it to make it as terse and precise as possible to convey the maximum amount of information via the minimal number of words while maintaining the maximum precision i.e. trying to make it the best "TLDR" possible. The total amount of time for all 13 PRs was 76 minutes.

The "Summary" was not used as the PR description, it was prepended to the offending machine-generated one. In reality, *most* of the words being referenced were not your own.

> I specifically and intentionally removed all the filler from my summary that you added in yours (the first draft was closer to yours in length and verbosity) 

Your "Summary" is ~79 words, and his is ~87, or am I looking at the wrong things?

You can't lament 8 words of "filler" while posting ~531 words of generated text.

> so that the reviewer (a person who supposedly knows the code already) recognizes the key code paths to allow for immediate context localization and then can delve further into the "meandering wall of text" (I don't find it "meandering", it's sequential, detailed and thorough, and I did edit it repeatedly for clarity prior to posting) for additional details/motivation only if he so chooses.
> 3. PR and Issue verbosity are also not only for the reviewer but for future historic mining of context, so that the detailed motivation, diagnostics and fix circumstances are preserved if there are layered bugs discovered later on. This is the reason I prefer the more verbose record - it is useful for the future investigation of "why did these idiots do that?" 5 years down the line (I've been doing this for 25+ years - it always happens eventually).
> 

This is true when the expository commentary is focused on *surprising* aspects that the diff doesn't already make clear.

If you're just posting what an LLM decides is relevant with some minor edits, then any future developer can simply prompt their own LLM if they want the same level of detail.

As an example, I cleared the commit message and asked an LLM:

```
can you describe the HEAD commit?
```

And I got back:

```
HEAD (ea5a59d81174, message dev) fixes how LLVM finds the DWARF unit that owns
a debug-info entry (DIE). It now walks the parent chain looking for a DIE
owned by a DIEUnit, rather than assuming the unit root has one of a short list
of tags. This lets entries in partial units resolve their unit and
section-absolute offsets correctly.

The commit also uses dwarf::isUnitType to validate unit tags and adds unit and
llvm-dwarfutil tests, including a cross-unit DW_FORM_ref_addr reference into a
partial unit.
```

Seems fine, and honestly if you had posted that with some minor edits I don't think anyone would bat an eye. I would still discourage this, and LLVM as a community still discourages this, but you would probably not waste maintainer time.

To try to reproduce what you originally posted, I asked it:

```
can you be more verbose, and include detailed descriptions of the dependent codepaths in a way that might help someone unfamiliar with the codebase understand what that means?
```

This got me to something closer to your first PR description.

There will always be judgement calls on the level of detail to get into, and what elements of a change are mundane vs. surprising, but lightly editing LLM output is not an acceptable approach.

> I readily accept criticism if it's substantive (that's why I added the summaries to provide a primer for TLDR) but if we're going to be haggling over the style of the prose - and to me this is about style of prose only for reasons provided in [2] - as opposed to the substance, I would rather maintain my own fork with the fixes - my time is also valuable and I can see it better spent automating patching the fork.
> 

This is not about the style of prose, it is about empathy, reciprocity, respect, and at a basic level before any of those: following community norms when it is pointed out that you are violating them.

To try to frame what is really at issue here in a more concrete way, I tried to summarize the interaction to this point:

* A maintainer politely indicates your PR description is too verbose, and seems LLM-generated, linking to a document describing the community norm you are violating.
* You reply that "I naturally didn't write it", seemingly making it clear you understand that it goes against documented community norms.
* You still decide to keep the *strongly discouragedly* generated contents of the PR and just *add* a hand-written summary.
* The maintainer tries again to advise you that adding more text to a description which is already too verbose is not what is being asked, and even takes the time to sketch out an example of the kind of commit message he is looking for.
* You push back pretty hard, for some reason; at least in my initial readings I take much of your response as antagonistic, but I imagine this is just a cultural or personal difference. I only mention this impression because I imagine many others will have registered the same, and I don't want to pretend I am not concerned about it.

You also off-handedly mention you "don't have such throughput capability" when describing the amount of machine-generated text you are posting against the documented wishes of the project, and against the request of the human maintainer that is trying to nudge you towards something acceptable. This gives the impression that you expect maintainers to devote more effort to reviewing your changes than you intend to put into writing them. That is the kind of "extractive" change @JDevlieghere is talking about, and LLVM as a community is not interested in those. I'm not accusing you of holding this belief, or trying to "extract" anything, but only trying to convey to you why this is an issue.

I can also land the change, but I would ask that you update the summary first. If you prefer not to, then you can wait for @JDevlieghere to land it.

I would be happy to review the other PRs you have open, once you've written reasonable PR descriptions for them by hand.

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