[cxx-interop] Inline constexpr vars. #35311
Conversation
|
I suppose I could create a benchmark for this now that we support C++ interop for benchmarks 😉 |
zoecarver
force-pushed
the
cxx/inline-constexpr-vars
branch
2 times, most recently
from
fc8c9f3 to
161e003
Compare
| @@ -209,8 +209,8 @@ void ASTScope:: | |||
| } | |||
|
|
|||
| void ASTScope::expandFunctionBody(AbstractFunctionDecl *AFD) { | |||
| auto *const SF = AFD->getParentSourceFile(); | |||
There was a problem hiding this comment.
I think the problem here is that we've never actually had a C++ function with a body before. And because there's no source file associated with a C++ decl context, it makes sense that this might be null.
|
@swift-ci please smoke test. |
|
@swift-ci please smoke test macOS. |
|
@swift-ci please smoke test Windows. |
zoecarver
force-pushed
the
cxx/inline-constexpr-vars
branch
from
161e003 to
4e099ae
Compare
If a static variable can be evaluated at compile time, create an accessor using that value. This means static variables will always be inlined and removed. Note: currently this only works with numeric types.
zoecarver
force-pushed
the
cxx/inline-constexpr-vars
branch
from
4e099ae to
6bc2696
Compare
|
@swift-ci please smoke test. |
|
@swift-ci please smoke test macOS. |
|
@swift-ci please smoke test Windows. |
|
Unrelated issue on Linux. @swift-ci please smoke test Linux. |
|
@swift-ci please smoke test Linux. |
4 similar comments
|
@swift-ci please smoke test Linux. |
|
@swift-ci please smoke test Linux. |
|
@swift-ci please smoke test Linux. |
|
@swift-ci please smoke test Linux. |
|
I'm going to go ahead and merge this shortly. |
| @@ -209,8 +209,8 @@ void ASTScope:: | |||
| } | |||
|
|
|||
| void ASTScope::expandFunctionBody(AbstractFunctionDecl *AFD) { | |||
| auto *const SF = AFD->getParentSourceFile(); | |||
| SF->getScope().expandFunctionBodyImpl(AFD); | |||
| if (auto *const SF = AFD->getParentSourceFile()) | |||
There was a problem hiding this comment.
This should be const auto * rather than auto * const which makes the pointer const. While the pointer is nice to make const, it is not as valuable as the value itself being marked const.
There was a problem hiding this comment.
Unfortunately, that doesn't work because getScope is not a const member (and can't be because it lazily creates the scope). So, I'm going to leave this as-is.
| @@ -209,8 +209,8 @@ void ASTScope:: | |||
| } | |||
|
|
|||
| void ASTScope::expandFunctionBody(AbstractFunctionDecl *AFD) { | |||
| auto *const SF = AFD->getParentSourceFile(); | |||
| SF->getScope().expandFunctionBodyImpl(AFD); | |||
| if (auto *const SF = AFD->getParentSourceFile()) | |||
There was a problem hiding this comment.
I think that this case deserves a comment to explain why/when we expect a source to be missing.
There was a problem hiding this comment.
Good point. Fixed in a follow up here: #35364.
Add a comment explaining that C++ function wont have an attached parent source file. Refs swiftlang#35311.
If a static variable can be evaluated at compile-time, create an accessor using that value. This means static variables will always be inlined and removed.
Note: currently this only works with numeric types.
Resolves SR-14023.