Moved resolveAliasThis from aliasthis.d to expressionsem.d

[AsterMiha]

To separate semantic elements from aliasthis.d

[dlang-bot]

Thanks for your pull request and interest in making D better, @AsterMiha! 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#11743"

[MoonlightSentinel]

Could you elaborate how this helps dmd as a library?
(dsymbolsem.d is already quite large and this code is explicitly dealing with alias this)

[RazvanN7]

@MoonlightSentinel The general idea is that we want to be able to have a clear separation between files required at parse time and files required for semantic analysis. Currently, to be able to do this we have the massive duplication of code in ASTBase. This is an effort to get rid of that duplication. At the end we are going to have files that contain AST nodes with their fields and simple methods that do not require semantic analysis and other files that contain functions that operate semantic analysis on the AST.

Although, it might be better to put the function in expressionsem.d since that is where it is mostly used.

[RazvanN7]

See comments below.

[RazvanN7]

The function below checkDeprecatedAliasThis should also be moved wherever resolveAliasThis goes and also it should be made private. There are 2 reasons for this: firstly, because it is a helper function and only resolveAliasThis calls it, secondly, because it also imports dsymbolsem and therefore has a dependency to a module that does semantic analysis.

[RazvanN7]

I think that it is a better fit to move resolveAliasThis to expressionsem.d. There are no calls to this function in dsymbolsem.d and the operations are performed on an expressions, so technically it could be seen as been part of semantic on an expression.

Also, putting it in expressionsem will also make some of the imports to dsymbolsem that you added unnecessary.

[Geod24]

The general idea is that we want to be able to have a clear separation between files required at parse time and files required for semantic analysis.

Why ? Is it because you want to provide the ability to have DMD as a library work as a parser only ? If so, don't bother with that. Or rather, not now. We already have libdparse, which was built as a library, and almost everyone uses it for parsing D code, because it's much easier to deal with. The value in DMD as a library is not in the parser, but in the post-semantic AST.

[RazvanN7]

Why ? Is it because you want to provide the ability to have DMD as a library work as a parser only ?

That is possible already, but you need to use ASTBase. If we want to get rid of that code duplication we must have this separation.

[RazvanN7]

Just to make things clear, we have 2 goals: (1) pull out semantic functions from AST nodes AND (2) get rid of ASTBase. Both goals are targeted to make the code base cleaner, however, (2) requires some changes that are orthogonal to (1). This PR is targeted towards achieving (2).

[Geod24]

(1) pull out semantic functions from AST nodes AND (2) get rid of ASTBase.

Alright, that is slightly different. And IMO ASTBase was the wrong approach to begin with, for the reason I mentioned above.
Anyway, if you want to get this merged, at least squash (and get rid of the merge commit).

[ibuclaw]

This caused a regression https://issues.dlang.org/show_bug.cgi?id=21294

[ibuclaw]

This caused another regression https://issues.dlang.org/show_bug.cgi?id=21295