[InterfaceGen] Print bodies of inlinable functions in textual interfaces #19224
Conversation
include/swift/AST/Decl.h
Outdated
There was a problem hiding this comment.
I know this isn't ready for review yet, but this should be able to go into the anonymous union below, right?
There was a problem hiding this comment.
Yep, good call!
harlanhaskins
force-pushed
the
extremely-inline
branch
2 times, most recently
from
252bbfd to
9889a12
Compare
include/swift/AST/Decl.h
Outdated
There was a problem hiding this comment.
Hm. Aren't these going to be mutually exclusive, like they are for default argument values?
There was a problem hiding this comment.
I guess I should add BodyKind::Deserialized to cover that, but yes it should be mutually exclusive. The check for this would have to be outside of Parsed/TypeChecked
include/swift/AST/Decl.h
Outdated
harlanhaskins
force-pushed
the
extremely-inline
branch
from
0b4b972 to
de1d642
Compare
harlanhaskins
changed the title
harlanhaskins
changed the title
harlanhaskins
force-pushed
the
extremely-inline
branch
from
a3c8389 to
f8e9a29
Compare
jrose-apple
left a comment
jrose-apple
left a comment
There was a problem hiding this comment.
Looks pretty good! Only minor comments.
There was a problem hiding this comment.
This one doesn't seem right; subscripts don't have bodies.
lib/AST/ASTVerifier.cpp
Outdated
There was a problem hiding this comment.
This should probably be in the top bucket, since "Deserialized" is closest to "None" rather than "Unparsed".
lib/AST/Decl.cpp
Outdated
There was a problem hiding this comment.
Maybe this should be a switch?
lib/Serialization/Serialization.cpp
Outdated
There was a problem hiding this comment.
This one won't actually ever appear in the SIL namespace, so you don't need to add it here.
lib/Serialization/Serialization.cpp
Outdated
There was a problem hiding this comment.
@slavapestov, this is the correct check for inlinable functions of any kind, right?
lib/Serialization/Serialization.cpp
Outdated
There was a problem hiding this comment.
David would probably say to add "maybe…" or "…IfNeeded" to the name of this method, since it doesn't always do anything.
There was a problem hiding this comment.
Can you add a CHECK-NEXT for the close brace?
There was a problem hiding this comment.
We're planning to change this to always use get {, right?
There was a problem hiding this comment.
Yep, should I go ahead and roll that into this patch?
There was a problem hiding this comment.
A follow-up is fine; just wanted to have it on record.
|
@swift-ci please test |
|
Build failed |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@harlanhaskins Why do we need to serialize the inlinable body text? I thought it was for swiftinterface files only |
|
@slavapestov We need to be able to compile an interface file into a binary swiftmodule, which means recompiling inlinable bodies |
There was a problem hiding this comment.
We should probably just never print @objc on deinit in textual interfaces. Somehow. But that can happen later; this is clever.
|
Ah, that's not really it: it's when you do a non-whole-module build but still want to generate a textual interface. In that case you pass through the partial module stage. |
|
By the way, Slava, here's the question I originally tagged you in on: |
harlanhaskins
force-pushed
the
extremely-inline
branch
from
82fb586 to
aa3859b
Compare
harlanhaskins
force-pushed
the
extremely-inline
branch
from
aa3859b to
23a7031
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
⛵ |
…ces (#19224) * Introduce stored inlinable function bodies * Remove serialization changes * [InterfaceGen] Print inlinable function bodies * Clean up a little bit and add test * Undo changes to InlinableText * Add serialization and deserialization for inlinable body text * Allow parser to parse accessor bodies in interfaces * Fix some tests * Fix remaining tests * Add tests for usableFromInline decls * Add comments * Clean up function body printing throughout * Add tests for subscripts * Remove comment about subscript inlinable text * Address some comments * Handle lack of @objc on Linux
4 participants
This patch adds support to the AST printer for printing the bodies of inlinable functions.
Still left to do: