Skip to content

Fixups for UnsafePointer documentation #5994

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

Merged
merged 3 commits into from
Dec 2, 2016
Merged

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Nov 30, 2016

Two minor fixups, and one big/scary documentation change.

Until now we've failed to document the Undefined Behaviour requirements on pointer offsets. In particular we lower these operations to LLVM's GetElementPointer inbounds: http://llvm.org/docs/LangRef.html#id214

If the inbounds keyword is present, the result value of the getelementptr is a poison value if the base pointer is not an in bounds address of an allocated object

In order to avoid getting into the nitty gritty details of poison, I just specified staying in bounds of the current allocation is a precondition. I am very far from certain that this is the ideal wording, and we should discuss the best wording (and how much Swift wants to inherit from llvm here).

@Gankra
Copy link
Contributor Author

Gankra commented Nov 30, 2016

CC @atrick @natecook1000 @bjlanier

@jrose-apple
Copy link
Contributor

Don't forget "just-past-the-end" pointers, which (like in C) are also valid.

Alexis Beingessner added 3 commits November 30, 2016 18:35
Offsets lower to GetElementPointer inbounds, which specifies that it's
Undefined Behaviour for the result to be out of bounds, regardless of if
the result is actually dereferenced.
@Gankra
Copy link
Contributor Author

Gankra commented Nov 30, 2016

@jrose-apple I didn't forget them, I was just hoping everyone in the world would assume this is included in the definition of in-bounds. I have a feeling the "correct" solution involves all of these preconditions linking to a definition, but I'm not sure where that would go.

@Gankra
Copy link
Contributor Author

Gankra commented Dec 1, 2016

@swift-ci please smoke test

@atrick
Copy link
Contributor

atrick commented Dec 1, 2016

Looks fine to me.

@natecook1000
Copy link
Member

TIL about poison values. LGTM! Going to merge so I can integrate these changes into some other revisions.

@natecook1000 natecook1000 merged commit df8486d into swiftlang:master Dec 2, 2016
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.

4 participants