remove HiddenParameters, refactor declareThis

[thewilsonator]

Also qualify the void* __capture context parameter.

@ibuclaw is that used anywhere in GDC? looks like it could be removed from the headers.

[dlang-bot]

Thanks for your pull request and interest in making D better, @thewilsonator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#10042"

[WalterBright]

Wasn't this added in the recent fix so two this pointers could be used? Was that all unnecessary, or were insufficient test cases added?

[thewilsonator]

Some stuff was added to it with that, but git blame says: #9179

cc @jacob-carlborg

[thewilsonator]

For reference this PR is part of me trying to get to the bottom of why closure's contexts are not typed (and type checked) properly.

[jacob-carlborg]

declareThis used to return one hidden this parameter. In one of the Objective-C PRs I added a second hidden parameter (because that's required by the Objective-C ABI). I think after that a third parameter was added to handle lambdas that both capture the this pointer and the enclosing function context.

[jacob-carlborg]

I think it's a bit difficult to follow the refactoring in this PR. What is better with this change?

In general I think it's better to have as few methods as possible that modify their parameters or the receiver. The previous version returned the result.

[thewilsonator]

The previous version returned the result.

... and then immediately assigned to those members.

What is better with this change?

line count change for one. HiddenParameters exists only as a temporary, there is no reason for it to exist.
This also unifies the way hidden variables are created/analysed reducing indentation and factoring common paths.

[jacob-carlborg]

and then immediately assigned to those members.

Yes. I still think it’s better. I’ve been trying to use DMD as a library now for a while and it’s a pain in the ass that the compiler changes things inline all over the place.

It would be much better if one method was responsible for one thing and didn’t modify values in place.

[ibuclaw]

Pretty sure that this can just be internal to the frontend.

[ibuclaw]

I have no problem with this if you think it's ready for inclusion.

[ibuclaw]

Note to self, this type could be cached somewhere for easy comparison in the backend where void*[2]* can be replaced with a two field struct with the actual context types as its fields.