Use more robust pattern matching in Expression_optimize.visitAddr

[nordlow]

[dlang-bot]

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

[nordlow]

There are loads of other similar refactorings possible. Shall I look into these aswell? If so at what granularity should I split these up? By file, by function or by match expression?

[dkorpel]

What is more robust about this? Test suite is failing.

[nordlow]

What is more robust about this? Test suite is failing.

Ok, I meant idiomatic. Gonna look into the test failure.

[RazvanN7]

@nordlow any progress on this?

[nordlow]

@nordlow any progress on this?

I can't see what's wrong with my refactoring.

[BorisCarvajal]

I can't see what's wrong with my refactoring.

You changed if (ae.e2.isIntegerExp() && ae.e1.isVarExp()) to if (auto ie = ae.e2.isIntegerExp()), but there is a else branch below that is not taken now.

[nordlow]

I can't see what's wrong with my refactoring.

You changed if (ae.e2.isIntegerExp() && ae.e1.isVarExp()) to if (auto ie = ae.e2.isIntegerExp()), but there is a else branch below that is not taken now.

Ahh, thanks. I've adjusted things at https://github.com/dlang/dmd/commit/e73143415949b99f8f880f1b05969e958e5577a1. Had to increase identation. It's a pity github doesn't do a better job of reducing the diff in these cases.

[dkorpel]

It's a pity github doesn't do a better job of reducing the diff in these cases.

There's a "Hide whitespace" option under the gear wheel. Still, I don't like this refactor, usually we want to reduce indentation. I don't see much wrong with the way it is now.

[nordlow]

It's a pity github doesn't do a better job of reducing the diff in these cases.

There's a "Hide whitespace" option under the gear wheel. Still, I don't like this refactor, usually we want to reduce indentation. I don't see much wrong with the way it is now.

My refactoring avoids redundant calls to isXXXExp functions.

If D supported a syntax similar to

if ((auto x = ...) && (auto y = ...))

we could avoid these extra levels of indentation.

[RazvanN7]

This looks like it has been abandoned and the need for this PR is doubtful anyway. I have added the 72h no response -> close label. @nordlow if you are able to make a case for this by then we can continue the discussion, otherwise I will close this.

[ljmf00]

Please rebase against master.

[RazvanN7]

Yes @nordlow , please rebase.