-
Notifications
You must be signed in to change notification settings - Fork 246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/initialize list side branch #6058
Open
kaizhangNV
wants to merge
39
commits into
shader-slang:master
Choose a base branch
from
kaizhangNV:feature/initialize-list-side-branch
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Feature/initialize list side branch #6058
kaizhangNV
wants to merge
39
commits into
shader-slang:master
from
kaizhangNV:feature/initialize-list-side-branch
+1,054
−213
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
requested review from
tangent-vector,
csyonghe,
expipiplus1 and
a team
as code owners
January 10, 2025 18:36
kaizhangNV
removed request for
a team,
expipiplus1,
csyonghe and
tangent-vector
January 10, 2025 18:37
kaizhangNV
force-pushed
the
feature/initialize-list-side-branch
branch
12 times, most recently
from
January 16, 2025 04:51
282ad96
to
8be343e
Compare
kaizhangNV
force-pushed
the
feature/initialize-list-side-branch
branch
6 times, most recently
from
January 17, 2025 05:16
2f3c992
to
7c5a8e8
Compare
We should not use parameter's default value in the synthesized member-init ctor. The default value should only be used when call site doesn't provide enough arguments, and our invoke system will use those default value to make up the missing arguments. Add '-vk' to few tests to enable vulkan backend test.
Fix gfx-smoke test, for non-C style struct, StructType a = {}; will never work anymore.
-Formatting
…ructor When successfully check the synthesized member initializer list constructor, we don't need the default constructor anymore, so we can safely remove it.
Disable -zero-initialize option and disable all the related tests. File shader-slang#6109 to track this issue.
Add cuda-host decoration for the synthesized constructor if there is any usage of TorchTensor.
When creating invoke expression for ctor, we need to call ResolveInvoke() to find us the best candidates, however the existing lookup logic could find us the base constructor for child struct, we should eliminate this case by providing the LookupOptions::IgnoreInheritance to lookup, this requires us to create a subcontext on SemanticsVisitor to indicate that we only want to use this option on looking the constructor. This change implements this.
Apply all the initializer list logic on core moduls structs as well. Add special case handling for backwards-compatibility feature for HLSL when type cast a literal zero to a 'struct'.
Instead of checking what is C-Style struct, we should check C-Style type. Proposal is not very clear about this. Will also update proposal.
kaizhangNV
force-pushed
the
feature/initialize-list-side-branch
branch
4 times, most recently
from
January 23, 2025 22:29
97a71c7
to
1788d3b
Compare
kaizhangNV
force-pushed
the
feature/initialize-list-side-branch
branch
from
January 24, 2025 01:20
1788d3b
to
55ea908
Compare
csyonghe
reviewed
Jan 24, 2025
@@ -12332,8 +12295,8 @@ void SemanticsDeclAttributesVisitor::_synthesizeCtorSignature(StructDecl* struct | |||
|
|||
// Only the members whose visibility level is higher or equal than the | |||
// constructor's visibility level will appear in the constructor's parameter list. | |||
List<VarDeclBase*> resultMembers; | |||
if (!_searchMembersWithHigherVisibility(structDecl, ctorVisibility, resultMembers)) | |||
List<KeyValuePair<VarDeclBase*, String>> resultMembers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is necessary for _searchInitializableMembers
to return the name of the enclosing struct, because you can easily obtain the struct decl from VarDeclBase->getParent
.
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.
No description provided.