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.

Sign up for GitHub

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

Jump to bottom

fix #21153 - [REG 2.111.0] Infinite loop in isAliasThisTuple #21155

Merged
merged 1 commit into from
Apr 8, 2025
Merged

fix #21153 - [REG 2.111.0] Infinite loop in isAliasThisTuple #21155

merged 1 commit into from
Apr 8, 2025

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Apr 5, 2025 •
edited
Loading

Fixes the issue by way of reverting the "refactoring" in #16868.

Partially reverts the regressing change in 0890136. The "fixed"
refactoring should be applied to master/development branch.

Because fixing the refactoring might just introduce more bugs.

@ibuclaw ibuclaw added Severity:Regression PRs that fix regressions Review:Trivial typos, formatting, comments Review:stable-priority A regression fix to stable that should receive higher priority labels Apr 5, 2025
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

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.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

  • In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

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 "stable + dmd#21155"

@ibuclaw ibuclaw linked an issue Apr 5, 2025 that may be closed by this pull request
[REG 2.111.0] Infinite loop in isAliasThisTuple #21153
Closed
thewilsonator
thewilsonator reviewed Apr 5, 2025
View reviewed changes
Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

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

How exactly does this fix the issue at hand?
This appears to just undo a desirable refactoring?

@ibuclaw
Copy link
Member Author

ibuclaw commented Apr 6, 2025 •
edited
Loading

Code doesn't work before -> code works after. That is exactly the definition of a bug fix.

The only desirable code is working code, it does not matter how it looks.

@ibuclaw
fix dlang#21153 - [REG 2.111.0] Infinite loop in isAliasThisTuple
d26ebe2 
Partially reverts the regressing change in 0890136. The "fixed"
refactoring should be applied to master/development branch.
@ibuclaw ibuclaw force-pushed the fix21153-by-revert branch from 698ff13 to d26ebe2 Compare April 6, 2025 10:00
thewilsonator
thewilsonator requested changes Apr 6, 2025
View reviewed changes
Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

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

Is #21154 insufficient? That is the one with the test case.

I am not inclined to merge this without one, is for no reason that it is liable to re-refactored.

@ibuclaw
Copy link
Member Author

ibuclaw commented Apr 8, 2025 •
edited
Loading

Is #21154 insufficient? That is the one with the test case.

I am not inclined to merge this without one, is for no reason that it is liable to re-refactored.

Who knows? This code worked just fine for many releases before it was refactored. The other PR likely should have targeted master, as it's a feature change where there might still be unknown things broken with the function.

Test is present as this is rebased.

@ibuclaw ibuclaw merged commit 51816cd into dlang:stable Apr 8, 2025
120 of 126 checks passed
@ibuclaw ibuclaw deleted the fix21153-by-revert branch April 8, 2025 14:27
@thewilsonator
Copy link
Contributor

Test is present as this is rebased.

What does that mean? There is no test in the diff of this PR. This should not have been merged.

hubot pushed a commit to gcc-mirror/gcc that referenced this pull request Apr 8, 2025
@ibuclaw
d: Fix infinite loop in isAliasThisTuple
6393761 
This reverts a change in the upstream D implementation of the compiler,
as the refactoring introduced a regression.

gcc/d/ChangeLog:

	* dmd/MERGE: Merge upstream dmd 51816cd01d.

Reviewed-on: dlang/dmd#21155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thewilsonator thewilsonator thewilsonator requested changes

@dkorpel dkorpel dkorpel approved these changes

Assignees
No one assigned
Labels
Review:stable-priority A regression fix to stable that should receive higher priority Review:Trivial typos, formatting, comments Severity:Regression PRs that fix regressions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[REG 2.111.0] Infinite loop in isAliasThisTuple
4 participants
@ibuclaw @dlang-bot @thewilsonator @dkorpel