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.

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

fix Issue 20041 - CTFE incorrect result with __vector #12057

Merged
merged 1 commit into from Dec 28, 2020

Conversation

WalterBright
Copy link
Member

Took me a lot longer that it should have to find the source of this problem. CTFE is much too complicated.

@dlang-bot
Copy link
Contributor

dlang-bot commented Dec 27, 2020

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
20041 critical CTFE incorrect result with __vector

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#12057"

@@ -38,6 +38,7 @@ import dmd.identifier;
import dmd.init;
import dmd.initsem;
import dmd.mtype;
import dmd.printast;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import dmd.printast;
debug import dmd.printast;

Or delete ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

I keep needing to add it back in. It doesn't hurt to leave it in.

Copy link
Member

Choose a reason for hiding this comment

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

With debug you don't have to remove it. It will be there just for your local debugging, but not compiled in the final release.

The same applies for all these printf lines, which could be prefixed with debug (ctfe) or something like that, so that you don't need to manually comment/uncomment them.

Copy link
Member

Choose a reason for hiding this comment

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

One of the new developers on my team (who was primary a TypeScript developer before joining the team), told me that his favorite D feature is the ability to do:

debug import std.stdio : writeln, writefln;

//...

debug
{
    // various writefln/writeln debug statements...
    writeln(...);
}

;)

@PetarKirov
Copy link
Member

PetarKirov commented Dec 27, 2020

All bug fixes should target the stable branch. (Unless they're so risky that can cause introducing new bugs, of course.)
This is a difference of 2 months between getting the bug fixed and getting the fixed compiler in the hands of users.

To target stable perform these two steps:

Let me expand the first step a bit, as I've seen people sometimes run into troubles with it:

  1. First check you git remotes. The assumption is that upstream points to this repo (so either https://github.com/dlang/dmd or git@github.com:dlang/dmd, depending on if you use ssh/git or https to access it) and origin points to your own fork of dmd. So, for you @WalterBright I'd expect something like this:

    git remote -v
    origin	git@github.com:WalterBright/dmd (fetch)
    origin	git@github.com:WalterBright/dmd (push)
    upstream	git@github.com:dlang/dmd (fetch)
    upstream	git@github.com:dlang/dmd (push)

    (A git remote is just an alias for a url. What name you give it doesn't matter, as long as you know which is which. E.g. in my
    case upstream is dlang and origin is me:

    git remote -v
    dlang	https://github.com/dlang/dmd (fetch)
    dlang	https://github.com/dlang/dmd (push)
    me	git@github.com:PetarKirov/dmd (fetch)
    me	git@github.com:PetarKirov/dmd (push)

    I use https, instead of git protocol for my dlang remote, in order to prevent accidental git push-ing to it.)

  2. Next make sure that your master and stable branches are up to date with this repo (so upstream , not origin):

    • For master:
      git checkout master
      git pull --ff-only upstream master
    • For stable:
      git checkout stable
      git pull --ff-only upstream stable

    If you get an error like this:

    hint: 'stable' matched more than one remote tracking branch.
    hint: We found 2 remotes with a reference that matched. So we fell back
    hint: on trying to resolve the argument as a path, but failed there too!
    ...
    

    it means that git doesn't know which stable branch to pick as there a multiple options, so you need to be more explicit:

    git checkout --track upstream/stable
    

    And if you get a message like this:

    fatal: A branch named 'master' already exists.

    It means you already had this branch, but it's still necessary to make sure it's tracking the right "upstream" branch:

    git checkout stable
    git branch --set-upstream-to upstream/stable    

  3. And finally rebase:

    # Checkout the branch of this pull request:
    git checkout fix20041
    
    # Make sure it's up-to-date with upstream/master
    git rebase upstream/master
    
    # Finally rebase it on stable:
    git rebase --onto upstream/stable upstream/master

This may sound like quite the process, but I'm just being extra verbose, to make sure we don't miss something.


Once you know that your git remotes are set correctly, you don't need to repeat all of the above - just the following will do:

git fetch upstream master stable
git rebase --onto upstream/stable upstream/master

@PetarKirov
Copy link
Member

PetarKirov commented Dec 27, 2020

That said, @WalterBright I'd be happy to do the rebasing for you if you don't want to mess with it now.

@WalterBright
Copy link
Member Author

@PetarKirov Thanks for the explicit instructions. I'll take you up on your kind offer to do it for me! :-)

@PetarKirov PetarKirov changed the base branch from master to stable December 28, 2020 07:21
@PetarKirov
Copy link
Member

@PetarKirov Thanks for the explicit instructions. I'll take you up on your kind offer to do it for me! :-)

All done! There was a merge conflict in test/runnable/testxmm.d, but it was quick to resolve.

@WalterBright
Copy link
Member Author

Apparently this relies on other fixes to master. So it should move back to master.

@WalterBright
Copy link
Member Author

@PetarKirov can you please update the rebase to stable instructions with your new version? Thanks!

@WalterBright WalterBright deleted the fix20041 branch December 30, 2020 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants