[NFC] SILFunctionArgument and ApplySite docs and comments. #18119
Conversation
|
@gottesmm I would appreciate your review. |
|
Looking at this real quick while I am waiting for xcode to install... |
docs/SILDevelopment.md
Outdated
| @@ -0,0 +1,159 @@ | |||
| # SIL Development | |||
There was a problem hiding this comment.
Why not follow the lead of the standard libraries programmer's manual: StandardLibraryProgrammersManual.md. This would then by SILProgrammersManual.md. Matching the name would make it really obvious that these are related concepts and make this document more discoverable.
docs/SILDevelopment.md
Outdated
| `ApplySite::getCalleeArgIndexOfFirstAppliedArg()` to translate the | ||
| apply's arguments into function convention arguments. | ||
|
|
||
| Convenience methods exist for the most common uses, so there is |
There was a problem hiding this comment.
It would be useful to have several small examples here of these convenience methods/common use cases. I don't think you state what those are anywhere.
There was a problem hiding this comment.
Or maybe I am confused? Not sure.
docs/SILDevelopment.md
Outdated
| Throughout the compiler, integer indices are used to identify argument | ||
| positions in several different contexts: | ||
|
|
||
| - `SILFunctionType` parameters. |
There was a problem hiding this comment.
For each of these can you provide links to the APIs for manipulated each one of these. An example of how to get this range of indices would be useful as well potentially.
docs/SILDevelopment.md
Outdated
| - The SIL function definition's callee-side `SILFunctionArguments`, | ||
| including indirect results. | ||
|
|
||
| - Caller-side "applied arguments" for `apply` and `try_apply` (vs. operands). |
There was a problem hiding this comment.
What is the difference in between an "applied argument" vs an operand? That is not defined anywhere.
docs/SILDevelopment.md
Outdated
| > often copied into parts of the code where they are no longer valid; and (c) | ||
| > unlike LLVM IR, SIL is not stable and will continue evolving. | ||
|
|
||
| ``` |
There was a problem hiding this comment.
"As an example, consider the following swift code:"
docs/SILDevelopment.md
Outdated
| One direct formal parameter and one direct formal result: | ||
|
|
||
| ``` | ||
| SILFunctionType(foo): $(0:T) -> @out T |
There was a problem hiding this comment.
I understand what the 0 here is for but this isn't a proper sil type, no? Can you use a proper sil type and use the prose instead? Or maybe have names like FirstArgType? Not sure.
docs/SILDevelopment.md
Outdated
| } | ||
| ``` | ||
|
|
||
| Ignoring conventions, the closure `foo` would map to the following indices... |
There was a problem hiding this comment.
"Ignoring the current compiler stage (and thus SILFunctionConventions)". It would make it clearer (for me anyways).
docs/SILDevelopment.md
Outdated
| apply %closure(%result: $*T) | ||
| ``` | ||
|
|
||
| The mapping between SILFunctionType and SILFunctionArguments depends |
There was a problem hiding this comment.
Maybe move this before the previous section? Then it would be clearer what you are trying to show.
docs/SILDevelopment.md
Outdated
|
|
||
| A common mistake is to directly map the ApplySite's caller-side | ||
| arguments onto callee-side SILFunctionArguments. This happens to work | ||
| until the same code is exposed to a `partial_apply`. Always use |
There was a problem hiding this comment.
What is the "bug" here that occurs by exposing to a partial apply.
There was a problem hiding this comment.
(that is I think the error should be explicit in the prose).
Document SILFunction and `apply` arguments. Create TBD headers to give the document some form.
Use consistent terminology, clarify comments, and group APIs. This matches the documentation in SILDevelopment.md.
|
@swift-ci smoke test. |
|
@gottesmm thanks for reading the rough draft. I'm pushing this now to avoid upcoming merge conflicts on the cleanup, and I'm out of time for writing docs. @eeckstein @shajrawi @aschwaighofer Please take a look at the SIL Programmers Guide. I want to make sure everyone working on SIL understands what's there and starts contributing to the doc. |
|
@atrick SGTM. What is important is that this is a first step. We can iterate. |
This is in response to recurring bugs, a recently noticed bug, and feedback that the APIs don't make sense.