[PATCH] D126224: Add DWARF string debug to clang release notes.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG011e0604ebc9: Add DWARF string debug to clang release notes. (authored by hctim). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D126224: Add DWARF string debug to clang release notes.

hctim updated this revision to Diff 437717. hctim marked an inline comment as done. hctim added a comment. Change a small amount of wording, rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126224/new/ https://reviews.llvm.org/D126224 Files:

[PATCH] D126224: Add DWARF string debug to clang release notes.

hctim marked an inline comment as done. hctim added a comment. In D126224#3534643 , @probinson wrote: > I see some unrelated whitespace changes, we generally don't like mixing those > with "real" changes. But the description seems fine to me. Looks lik

[PATCH] D126224: Add DWARF string debug to clang release notes.

hctim added a comment. (thanks for the bump - this one fell off the radar) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126224/new/ https://reviews.llvm.org/D126224 ___ cfe-commits mailing list cfe-comm

[PATCH] D126224: Add DWARF string debug to clang release notes.

hctim added a comment. Yeah, I'll look at landing it today. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126224/new/ https://reviews.llvm.org/D126224 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D126224: Add DWARF string debug to clang release notes.

paulkirth added a comment. so do we want to land this? or are there some outstanding issues I'm unaware of? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126224/new/ https://reviews.llvm.org/D126224 ___

[PATCH] D126224: Add DWARF string debug to clang release notes.

probinson added a comment. I see some unrelated whitespace changes, we generally don't like mixing those with "real" changes. But the description seems fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126224/new/ https://reviews.llvm.org

[PATCH] D126224: Add DWARF string debug to clang release notes.

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/docs/ReleaseNotes.rst:447 + structures *without* a ``DW_AT_name`` field, which is valid DWARF, but may + lead to assertion failures in some so

[PATCH] D126224: Add DWARF string debug to clang release notes.

hctim added a comment. It sounds like the consensus is what's represented in the release notes, "if this new feature breaks you, then you need to fix yourself". Does anyone have any concerns with the specific text, or does it LGTY? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D126224: Add DWARF string debug to clang release notes.

probinson added a comment. In D126224#3532769 , @dblaikie wrote: > While the string would be deduplicated, the offset in the DIEs (or index in > DWARFv5, plus offset in .debug_str_offsets) for each of these strings would > not. But perhaps that would st

[PATCH] D126224: Add DWARF string debug to clang release notes.

dblaikie added a comment. In D126224#3531949 , @rnk wrote: > I am reminded of the perennial problem of "optional" protobuf fields that, > when omitted, will cause production crashes. > > Do you think it would be less disruptive to synthesize a name? I be

[PATCH] D126224: Add DWARF string debug to clang release notes.

probinson added a comment. Unnamed variables are an oddity, sure; we've had to patch a downstream test or two that wasn't being careful enough. But not providing a name is entirely defensible, and consumers should be willing to cope with DWARF that doesn't fully meet their expectations. Repo

[PATCH] D126224: Add DWARF string debug to clang release notes.

paulkirth added a comment. I'm going to try to summarize an offline discussion w/ @mcgrathr about this here: There are some other considerations to think about w.r.t. emitting names for non-source language constructs, as would be the case here. In fact, DWARF already handles this case: the "th

[PATCH] D126224: Add DWARF string debug to clang release notes.

paulkirth added a subscriber: mcgrathr. paulkirth added a comment. @rnk the standard even has an example of this exact behavior, so I think it's hard to say its //wrong// for LLVM to do this, but that may be more gentle. I'm going to defer to more expert opinions here, however, and loop in @mcgr

[PATCH] D126224: Add DWARF string debug to clang release notes.

rnk added a comment. I am reminded of the perennial problem of "optional" protobuf fields that, when omitted, will cause production crashes. Do you think it would be less disruptive to synthesize a name? I believe the string lives in a string pool, so naming all string literals `` seems like i

[PATCH] D126224: Add DWARF string debug to clang release notes.

hctim created this revision. hctim added reviewers: paulkirth, dblaikie. Herald added a project: All. hctim requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. D12353 added inline strings to the DWARF info pro