Skip to content

Bugfix for missing semaphore in typed template parameters. #918

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

Merged
merged 8 commits into from
Jan 16, 2024

Conversation

MaxSagebaum
Copy link
Contributor

@MaxSagebaum MaxSagebaum commented Jan 6, 2024

Resolves #877

Minimal example: https://cpp2.godbolt.org/z/Keozos3ae

x: <Ts...: int> type = { 
    func: () = {}
}

Error:

main.cpp2:2:32: error: pack expansion does not contain any unexpanded parameter packs
    2 |     template <int Ts> auto x<Ts...>::func() -> void{}

Correct code:

template <int ... Ts> auto x<Ts...>::func() -> void{}

I searched the issues for ..., variadic, semaphore and pack but could not find one that addresses this error.

@JohelEGP
Copy link
Contributor

JohelEGP commented Jan 6, 2024

That would be #877.

@MaxSagebaum
Copy link
Contributor Author

Thank you for the hint. I checked your reproducer with this version but my bugfix does not solve the issue.

I still get the error:

pure2-variadics.cpp2:10:20: error: declaration of template parameter ‘_’ shadows template parameter
pure2-variadics.cpp2:9:10: note: template parameter ‘_’ declared here
pure2-variadics.cpp2:10:33: error: declaration of template parameter ‘_’ shadows template parameter
pure2-variadics.cpp2:10:13: note: template parameter ‘_’ declared here
pure2-variadics.cpp2:10:62: error: expansion pattern ‘_’ contains no parameter packs
pure2-variadics.cpp2:10:65: error: template argument 1 is invalid
pure2-variadics.cpp2:10:59: error: too many template-parameter-lists

I might have a look at #877 during the next week. It should be located in the same part of the lowering.

@JohelEGP
Copy link
Contributor

JohelEGP commented Jan 6, 2024

That's because it's 2 issues in 1.
You can edit the opening comment with "Resolves #877",
and I will open a new one for the remaining issue.

@MaxSagebaum
Copy link
Contributor Author

Ok. I updated the opening comment.

Copy link
Contributor

@JohelEGP JohelEGP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@MaxSagebaum
Copy link
Contributor Author

MaxSagebaum commented Jan 11, 2024

I had time today to check the other error. It turned out, that this raised a few other problems.

930e70a : Fixes the missing name generation for typed template parameters. E.g.:

t: @struct <T...: _> type = { 
  f: <P: int> () -> i32 = 0;
}

0955521: Fixes the name clash for unnamed template parameters in different hierarchies. E.g.:

t: @struct <_...> type = { 
  f: <_: int> () -> i32 = 0; // Name for t<_> and t<_>::f<_> clashes both are 'UnnamedTypeParam1'
}

9612577: Fixes the missing name generation of unnamed parameters in template parameter declaration of a type. E.g.:

t: @struct <_...> type = { 
  f: <_: int> () -> i32 = 0; // Name for t<_> and t<_>::f<_> clashes both are 'UnnamedTypeParam1'
}

generates

template <auto ...UnnamedTypeParam1_26_14> template<int UnnamedTypeParam1_27_7> [[nodiscard]] auto t2<_...>::f() -> cpp2::i32 { return 0;  }

4bc819b: Updates the other test results. (This was less than I expected.)

I do not know why the tests fail on some machines. Is this something, that has been fixed in other branches? Should I do a rebase on master?

This solves now #877 fully. I could not find any other issues this would solve. I searched for unnamed. (But probably I missed them.)

@MaxSagebaum MaxSagebaum force-pushed the bugfix/missing-semaphore branch from 79806de to 6e33f5c Compare January 12, 2024 08:05
@MaxSagebaum
Copy link
Contributor Author

@hsutter @JohelEGP I fixed now the failures in the tests. So this pull request should now be ready for merge.

6e33f5c: Fixes also tests that are not part of this pull requests. This should clean up the testing pipeline.

@hsutter
Copy link
Owner

hsutter commented Jan 16, 2024

Thanks!

@hsutter hsutter merged commit 86064d2 into hsutter:main Jan 16, 2024
@jarzec
Copy link
Contributor

jarzec commented Jan 16, 2024

@hsutter After merging this PR regression tests for all compilers require an update. I will prepare a PR with updates for all GCC and Clang versions. However, due to issues with GitHub Windows runners, I am unable to update MSVC tests - I don't have MSVC installed on my machine. Could you please update the MSVC tests on main yourself?

@hsutter I've just noticed that the issue was mainly with the generated cpp files, not so much with the test-results. #941
should fix everything.

@JohelEGP

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] NTTP can't be variadic or unnamed
4 participants