SP004: implement initialize list translation to ctor #5215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kaizhangNV
force-pushed
the
feature/initialize-list
branch
3 times, most recently
from
b4079bc
to
c759f4d
Compare
kaizhangNV
force-pushed
the
feature/initialize-list
branch
from
04c9a9f
to
65fa60d
Compare
csyonghe
reviewed
Oct 3, 2024
kaizhangNV
force-pushed
the
feature/initialize-list
branch
from
65fa60d
to
0e8aa84
Compare
|
I rebase this PR to my another PR #5231 which just refactor the logic of synthesizing the body of the default constructor. This will make my next implementation easier. |
kaizhangNV
force-pushed
the
feature/initialize-list
branch
2 times, most recently
from
443609f
to
51b7311
Compare
kaizhangNV
force-pushed
the
feature/initialize-list
branch
5 times, most recently
from
f485378
to
b8094bd
Compare
This is the first change to implement part of SP004, what this
change does:
1. We synthesize a member-wise constructor for each struct follow
the rules described in SP004.
2. Add logic to translate the initialize list to constructor invoke
TODO: what needs to be done next
1. We didn't fill the body of the synthesized constructor
- This is relative easy task.
2. The fall back to legacy initialize list logic is not done
- I found out that it's straight forward to put this logic after
ResolveInvoke() call, because after ResolveInvoke() we will need to
do a final 'coerce()' check to determine whether the candidate is
applicable to use. If that fails, we can do the C-Style struct check
and fall back to legacy logic.
3. Should we enable this for stdlib as well?
- Our stdlib has some syntax that external users cannot use, so whether this
change could break anything is unknown for now. It needs some extra
effort to investigate and debugging the potential compile issue.
Separate the logic of _invokeExprForSynthesizedCtor from _readAggregateValueFromInitializerList. Because we don't really need to read and coerce the values recursively from the initializer list when we go this path. After we form a invoke expression to synthesized ctor, the ResolveInvoke logic will help us to coerce the arguments.
After we create the constructor invoke expression, we need to resolve this expression by ResolveInvoke(), and if it's failed we need a mechanism to fallback to the legacy initialization list logic. This change use sub visitor with a temporary diagnose sink to call the ResolveInvoke(), such that compile process won't be aborted if the error happens. The condition allowing the fall back is that the struct has to be a C-style struct, described in: https://github.com/shader-slang/slang/blob/master/docs/proposals/004-initialization.md
When searching the menbers of base type, we don't pick the fields of base type, instead, we will pick from parameter list of base type's constructor.
Don't report error when checking synthesized constructor, we will remove this constructor instead. Change the location where we create the member initialize constructor signature to SemanticsDeclAttributesVisitor to avoid the circular checking problem.
1. When extension of a struct defines __init, we should not synthesize __init anymore. 2. For the synthesized __init signature, we correct the default value for the parameter. Figuring out what exactly is "default initialize type". If a type has a ctor can take 0 parameter, it's default initialize type.
Struct Inheriting from a interface doesn't violate the rules of C-Style struct. Add the visibility modifier to the synthesized ctor.
kaizhangNV
force-pushed
the
feature/initialize-list
branch
from
cb9831a
to
1013d73
Compare
kaizhangNV
force-pushed
the
feature/initialize-list
branch
from
5a1ab65
to
406bc53
Compare
We need to ensure the definite checked for a struct before trying to use its constructor in the initialize list coerce.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First PR for #5192.
This is the first change to implement part of SP004, what this change does:
We synthesize a member-wise constructor for each struct follow the rules described in SP004.
Add logic to translate the initialize list to constructor invoke
TODO: what needs to be done next