Skip to content
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

Cleanup the hack code for the default ctor overload #5138

Open
kaizhangNV opened this issue Sep 23, 2024 · 0 comments
Open

Cleanup the hack code for the default ctor overload #5138

kaizhangNV opened this issue Sep 23, 2024 · 0 comments
Assignees
Labels
goal:forward looking Feature needed at a later date, not connected to a specific use case.

Comments

@kaizhangNV
Copy link
Contributor

kaizhangNV commented Sep 23, 2024

Backgroud:

This sub-task is split from #3406.

I wrap up Arial's PR #4854 to a new PR #5107 with some minor issues solved.

However, there still be lots of comments could potential involve large change on this PR.

We should not make one PR has infinitely long conversion and unlimited problems solved, so this task is create to only spot this issue.

We have a big issue in the implementation of the default ctor, where if there is no initExpr associated members or IDefaultInitializable inheritance in a struct, we will delete the ctor to avoid some warnings. This will bring us a bigger issue that we have to implement some hack code to overload the constructor invoke expression, e.g.

struct S { int x; }
S s = S();

For the above code, because we remove the default ctor, it means that S() is unavailable anymore. This will throw compiler error that "S() is not found". So we implement some hacky code to deal with this issue
https://github.com/shader-slang/slang/pull/5107/files/2b6f7a26688b60a74fde979fbfff28a24a493e5c#diff-2eecdb041b541df14ab93a0cfe35b4e0fbbcb63ef7fdcb004401829bfacdeb77R2400.

The original motivation to delete the ctor is to handle this case

struct S { int x; }
S s;

compiler will report a warning that variable x is not initialized, the reason is because x has neither initEpxr nor inherits from IDefaultInitializable, our synthesized default ctor won't initialize x.

This is not a good reason to delete the default ctor, we should either default very member in the default ctor no matter it has an initExpr.

This sub-task will track the effort to make the PR achieve this goal.

Specifically, the high level solution should be:

  1. during visitAggTypeDecl, always make sure to visit all the members first.
  2. During visitVarDecl, if we are visiting a member field declaration, we check if the member field has a initExpr or if the member field's type is IDefaultInitializable, if so, mark the parent decl NeedDefaultCtor.
  3. During visitAggTypeDecl, if the decl has been marked NeedDefaultCtor, proceed to synthesize the default ctor. (Nor for now)
  4. When synthesizing the default ctor, always intiailize every member, even if they don't have initExpr.

We should make sure that the step 1, 2 and 4 be implemented in this task.

@kaizhangNV kaizhangNV self-assigned this Sep 23, 2024
@bmillsNV bmillsNV added this to the Q4 2024 (Fall) milestone Sep 26, 2024
@bmillsNV bmillsNV added the goal:forward looking Feature needed at a later date, not connected to a specific use case. label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal:forward looking Feature needed at a later date, not connected to a specific use case.
Projects
None yet
Development

No branches or pull requests

2 participants