This is an archive of the discontinued LLVM Phabricator instance.

[Clang][Doc] Edit the Clang release notes
ClosedPublic

Authored by royjacobson on Jan 25 2023, 1:06 PM.

Details

Summary

Do a large edit of the Clang release notes.

This is meant to be merged into the Clang 16.x branch.

Diff Detail

Event Timeline

royjacobson created this revision.Jan 25 2023, 1:06 PM
royjacobson requested review of this revision.Jan 25 2023, 1:06 PM
royjacobson edited the summary of this revision. (Show Details)Jan 25 2023, 1:08 PM
royjacobson added a subscriber: Restricted Project.
rymiel added a subscriber: rymiel.Jan 25 2023, 2:16 PM

I don't know sphinx enough to know if the trailing underscore is needed in links, but there's about 5 links that don't have the underscore (Search for

>`)

)

clang/docs/ReleaseNotes.rst
570
810
812

A handful of suggestions as I spot-checked. This looks great, thank you for doing this! I'd like Aaron to take a look though.

clang/docs/ReleaseNotes.rst
46

no-ops read awkwardly to me, so I'm hopeful that 'silently ignored' is a little more descriptive.

Perhaps, "deprecated and ignored" if that isn't accurate?

48–51
49
166
  • Address comments from Erich and Emilia
  • Validate that the rst actually builds an html file
  • Fix some missing backticks and underscores

Thanks Erich and Emilia!

The html file is attached here, I think it's probably easier to proof read that way. (Took me a bit to figure out the CMake magic...)

I think it's worthwhile to mark comments with the "Done" checkmark in Phrabricator if they have been addressed

royjacobson marked 4 inline comments as done.Jan 27 2023, 4:12 AM
aaron.ballman added inline comments.Jan 27 2023, 9:25 AM
clang/docs/ReleaseNotes.rst
447–449

Assuming https://github.com/llvm/llvm-project/issues/60337 gets ported to the 16.0 branch, this bullet can be removed entirely.

aaron.ballman added inline comments.Jan 28 2023, 4:59 AM
clang/docs/ReleaseNotes.rst
447–449
tschuett added a subscriber: tschuett.EditedJan 28 2023, 5:25 AM

I am not the only one who has a different background. If the introduction says this is the introduction of the release notes, I am not super motivated to read on. I would prefer to see some highlights to motivate the readers to continue reading.

I am not the only one who has a different background. If the introduction says this is the introduction of the release notes, I am not super motivated to read on. I would prefer to see some highlights to motivate the readers to continue reading.

You've mentioned what I think are separate issues - that this document is difficult to read for readers from different backgrounds and that the introduction might be too long or unengaging.

I mostly agree about the first point - I've tried to do what I could to make it more accessible but I don't think it's enough.

About the second point - I'm not really sure what to change. I think the 'potentially breaking changes' section is more important than the 'what's new' section, but the 'what's new' section IS the second section (and there's a table of contents). Are there specific topics you think we should remove from the introduction or move into the intro from the 'what's new' section? I'd like to avoid duplicating information if we can.

IDK. Clang 16 is fully is a fully conformant C++20 except for some DRs. Or beginning with Clang 16, we start a long-term project to overhaul the diagnostics.

I agree that breaking changes are important.

  • Remove redundant paragraph from the intro
  • Remove outdated bug fix bullet
royjacobson marked 5 inline comments as done.Jan 28 2023, 8:53 AM
royjacobson added inline comments.
clang/docs/ReleaseNotes.rst
447–449

done.

royjacobson marked an inline comment as done.Jan 29 2023, 5:19 AM

IDK. Clang 16 is fully is a fully conformant C++20 except for some DRs. Or beginning with Clang 16, we start a long-term project to overhaul the diagnostics.

I agree that breaking changes are important.

I contemplated on this for a bit, and I don't have anything that I feel is representative enough and fits into one paragraph. I'm still not against the general idea though.

Thank you for working on this, it is really shaping up nicely!

IDK. Clang 16 is fully is a fully conformant C++20 except for some DRs. Or beginning with Clang 16, we start a long-term project to overhaul the diagnostics.

I agree that breaking changes are important.

I contemplated on this for a bit, and I don't have anything that I feel is representative enough and fits into one paragraph. I'm still not against the general idea though.

I think "major announcements" should be limited to things almost everyone will want to know about, like "we're changing our license" or "we've finished support for <major language revision>". So not every release will have major announcements, and that's fine -- we can remove any empty sections from the release notes.

clang/docs/ReleaseNotes.rst
16

Are you planning to merge the changes in generic text over to the trunk once this lands?

43

Do we want to mention that this may also impact build-time performance of projects relying on the default language standard mode, because C++17 pulls in a lot more compile-time-intensive APIs like constexpr and more template metaprogramming?

46

I don't know if we want to standardize on Clang-15 or Clang 15, etc. Feel free to edit as you see fit.

royjacobson marked 2 inline comments as done.

15 -> Clang 15

clang/docs/ReleaseNotes.rst
16

Yeah, absolutely!

43

I agree this change is annoying and has real life impact - I'm not trying to minimize it - but I think newer language versions bring enough benefits that we shouldn't warn users about using them in the Clang release notes. Especially since this doesn't come with any balancing context (like 'you can now use template variables instead of instantiating a struct every time you check if two types are equal' ;) )

46

Changed to Clang 15

aaron.ballman added inline comments.Jan 30 2023, 11:38 AM
clang/docs/ReleaseNotes.rst
43

That's fair! I was thinking of it from the perspective of whether we'll get bug reports or not. When we made the switch in-tree, some folks thought we introduced a compile time perf regression bad enough to warrant reverting the changes, which made me think users may probably think the same thing.

However, on reflection, I doubt that mentioning it in the release notes will actually mean less bug reports about it. :-P

Rebase to include the RISC-V changes.

royjacobson marked 2 inline comments as done.Feb 12 2023, 10:28 AM
royjacobson added a subscriber: Restricted Project.

friendly ping :)

aaron.ballman accepted this revision.Feb 13 2023, 5:18 AM

LGTM, thank you for this, the new layout is a big improvement!

This revision is now accepted and ready to land.Feb 13 2023, 5:18 AM