Allow spelling out a C type in the convention attribute. #28479
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
|
Kinda' neat that the difference in numbers between this PR and the clang function types in AST PR is exactly 100. |
lib/Parse/ParseDecl.cpp
Outdated
| consumeToken(tok::identifier); | ||
|
|
||
| // TODO: (Varun) The parser should probably not know about what conventions | ||
| // might have Clang types :(. |
There was a problem hiding this comment.
I don't think it's unreasonable, but you can certainly parse it unconditionally if you want and then diagnose if it's provided multiple times.
There was a problem hiding this comment.
I don't understand what you mean by "provided multiple times". The change I had in mind here was to get rid of the convention.Name checks.
There was a problem hiding this comment.
If you're going to parse generically, you should probably accept an arbitrary list of <identifier> : <expression> pairs and then complain about the ones you see multiple times or which you don't understand. I guess you don't have to, though.
There was a problem hiding this comment.
Oh, but one advantage of diagnosing this here rather than in Sema is that you can easily suppress follow-on diagnostics from Sema about a missing cType: clause by just dropping the attribute entirely.
There was a problem hiding this comment.
If you're going to parse generically
That is something I was trying to avoid. We can do that when we need it, instead of right now.
you can easily suppress follow-on diagnostics from Sema about a missing cType: clause by just dropping the attribute entirely
Not sure why we would diagnose if cType: is missing. We don't expect users to write the cType: -- would this be a diagnostic specific to new swiftinterfaces?
There was a problem hiding this comment.
I think we should at least require it in SIL types.
There was a problem hiding this comment.
Ok... but isn't the parser the wrong place to be doing a "error only if the convention is this or that and we are parsing SIL"? It seems convenient but...
There was a problem hiding this comment.
That should definitely be delayed until Sema.
For example, one may write a calling convention like convention(c, cType: "void *(void)") for a procedure that takes no arguments.
|
@swift-ci please test |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thanks! Just re-running tests in case: @swift-ci please test |
|
Build failed |
|
@swift-ci please clean test macOS platform |
|
Build failed |
|
Merging because it was only the Python linter which was failing, which has since been fixed. |
Once this PR + the #27479 PR land, I will land a third PR that includes the
StringRef <-> Clang Typeconversions. For now, we treated the string literal containing the type as some opaque data.Q. Should we special-case the "c" and "block" calling conventions for the
cTypekey (I am doing that right now)? It feels like a layering violation; the parser shouldn't know what string corresponds to C conventions vs Swift conventions. I implemented it this way because it seemed easy.