Skip to content

Remove doc comments from concrete floating-point types #26399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

stephentyrone
Copy link
Contributor

... when they are already present on the protocol. I don't think that we need them anymore for xcode documentation purposes. There are reasonable arguments both ways on this:

  1. when you're editing the concrete implementations, it's sometimes nice to have the doc comment right there.
  2. but it needlessly repetitive, and introduces the opportunity for comments to get out of sync.
  3. it also adds noise; it would be nice for information density if the implementation only had implementation notes.

Also includes some minor formatting fixups, and a bugfix for isCanonical on platforms that do not support subnormals (armv7, currently, which is sufficiently minor that I'm OK with stealth fixing it like this).

... when they are already present on the protocol. I don't *think* that we need them anymore for xcode documentation purposes. There are reasonable arguments both ways on this:
1. when you're editing the concrete implementations, it's sometimes nice to have the doc comment right there.
2. but it needlessly repetitive, and introduces the opportunity for comments to get out of sync.
3. it also adds noise; it would be nice for information density if the implementation only had implementation notes.
@stephentyrone
Copy link
Contributor Author

@swift-ci please test

In particular, document that subnormal encodings are treated as non-canonical zeros on platforms that flush to zero.
This one isn't a protocol requirement, so I was slightly overzealous removing it.
@stephentyrone
Copy link
Contributor Author

@swift-ci please smoke test

@stephentyrone
Copy link
Contributor Author

@natecook1000 is on vacation this week, I'm going to wait to merge this until he has a chance to give it at least a cursory approval.

Copy link
Member

@natecook1000 natecook1000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, with some minor nits! I’m especially glad to see the repeated docs dropping off the extension methods. Do we have tests that show that the docs are being correctly inherited from the protocols down onto the concrete symbols?

@stephentyrone
Copy link
Contributor Author

stephentyrone commented Aug 6, 2019

Do we have tests that show that the docs are being correctly inherited from the protocols down onto the concrete symbols?

I don't think this is testable for Swift, since it's an Xcode thing, but it seems to work robustly in 11. In the documentation view, you get a little:

Note
This documentation comment was inherited from FloatingPoint.

@natecook1000
Copy link
Member

Interesting, I hadn’t noticed that!

@stephentyrone
Copy link
Contributor Author

@swift-ci please smoke test

@stephentyrone stephentyrone merged commit fb2128b into swiftlang:master Aug 6, 2019
@stephentyrone stephentyrone deleted the floating-point-duplicate-comments branch August 6, 2019 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants