April 02, 2014
The case was made that it is absolutely necessary to retain the release branches in order to simplify preparation for point releases. And, while I initially argued against it, I do not see how it is possible to create the point release without doing so. However I strongly believe that resolutions to these regressions should first be implemented the oldest supported version (currently 2.065.1) and then merged forward to master. My reasoning? Read on!

I conducted a search on bugzilla for all regressions RESOLVED as FIXED after 2014-02-23 using the following:

http://d.puremagic.com/issues/buglist.cgi?bug_severity=regression&bug_status=RESOLVED&chfield=resolution&chfieldfrom=2014-02-23&chfieldto=Now&chfieldvalue=FIXED&columnlist=changeddate%2Cbug_severity%2Cpriority%2Cop_sys%2Cassigned_to_realname%2Cbug_status%2Cresolution%2Cshort_desc&query_format=advanced&resolution=FIXED&order=changeddate%2Cresolution%2Cassigned_to%2Cbug_id&query_based_on=

I figured I'd cherry-pick each into the 2.065 branch (starting the the oldest) as long as they were marked as regressions to v2.065.0 or older. Note that this approach does not correctly identify all regressions that needs to be included in a point release (see issue #12264 which is also fixes #12262). Anyway, I commenced by checking out the branch and issuing the command:

     git cherry-pick -m 1 53bec85

This specific pick is the merger of pull request #3339 which resolves (Issue 12243 - "ICE: cannot append 'char' to 'string'" with -inline). Immediately I encountered a merge conflict so I investigated the resolution and came across the following:

The pull essentially changes lines 1420 through 1425 of src/inline.c [a part of visitCallExp(CallExp*, Expression*) implementation]

 From ---------->

         if (eresult && e->type->ty != Tvoid &&
             !e->type->equals(eresult->type) &&
             eresult->type->hasWild() && !e->type->hasWild())
          {
             eresult = eresult->copy();
             eresult->type = e->type;
          }

To ---------->

         if (eresult && e->type->ty != Tvoid)
          {
             Expression *ex = eresult;
             while (ex->op == TOKcomma)
             {
                 ex->type = e->type;
                 ex = ((CommaExp *)ex)->e2;
             }
             ex->type = e->type;
          }

So far so good. Now cross-reference this to the version of src/inline.c from v2.065.0 and you will find that the function visitCallExp() does not exist. Further the function resident at the same location in the older version of the file [Expression * SliceExp::inlineScan(InlineScanState *iss){}] does not exist anywhere in the newer version (neither is any of a number of inlineScan() related implementations surrounding it). As such, it is not possible to pick this fix in preparation for v2.065.1.

What is the proper resolution for this problem in the context of the 2.065 branch?




April 03, 2014
There has been a _lot_ of code churn since the release was branched, and hopefully there will be even more shortly after the next release. Now is certainly not the easiest time to start backporting regression fixes.

That said, InlineScanVisitor::visitCallExp is equivalent to CallExp::inlineScan.  Working on line numbers is not going to help you here, and neither is plain function names.  Depending on how far you want to go, the pull request that did the refactoring should be obvious in git blame and _should_ make it clear where everything was moved to.  A 'cheat' option is to select a small chunk of old code and see if it appears in the new version.  Comments are especially good for this as they are likely to survive visitor refactorings unchanged.

Or, ask the pull author to backport.

On Thu, Apr 3, 2014 at 1:33 PM, Andrew Edwards <edwards.ac@gmail.com> wrote:
> The case was made that it is absolutely necessary to retain the release branches in order to simplify preparation for point releases. And, while I initially argued against it, I do not see how it is possible to create the point release without doing so. However I strongly believe that resolutions to these regressions should first be implemented the oldest supported version (currently 2.065.1) and then merged forward to master. My reasoning? Read on!
>
> I conducted a search on bugzilla for all regressions RESOLVED as FIXED after 2014-02-23 using the following:
>
> http://d.puremagic.com/issues/buglist.cgi?bug_severity=regression&bug_status=RESOLVED&chfield=resolution&chfieldfrom=2014-02-23&chfieldto=Now&chfieldvalue=FIXED&columnlist=changeddate%2Cbug_severity%2Cpriority%2Cop_sys%2Cassigned_to_realname%2Cbug_status%2Cresolution%2Cshort_desc&query_format=advanced&resolution=FIXED&order=changeddate%2Cresolution%2Cassigned_to%2Cbug_id&query_based_on=
>
> I figured I'd cherry-pick each into the 2.065 branch (starting the the oldest) as long as they were marked as regressions to v2.065.0 or older. Note that this approach does not correctly identify all regressions that needs to be included in a point release (see issue #12264 which is also fixes #12262). Anyway, I commenced by checking out the branch and issuing the command:
>
>     git cherry-pick -m 1 53bec85
>
> This specific pick is the merger of pull request #3339 which resolves (Issue 12243 - "ICE: cannot append 'char' to 'string'" with -inline). Immediately I encountered a merge conflict so I investigated the resolution and came across the following:
>
> The pull essentially changes lines 1420 through 1425 of src/inline.c [a part of visitCallExp(CallExp*, Expression*) implementation]
>
> From ---------->
>
>         if (eresult && e->type->ty != Tvoid &&
>             !e->type->equals(eresult->type) &&
>             eresult->type->hasWild() && !e->type->hasWild())
>          {
>             eresult = eresult->copy();
>             eresult->type = e->type;
>          }
>
> To ---------->
>
>         if (eresult && e->type->ty != Tvoid)
>          {
>             Expression *ex = eresult;
>             while (ex->op == TOKcomma)
>             {
>                 ex->type = e->type;
>                 ex = ((CommaExp *)ex)->e2;
>             }
>             ex->type = e->type;
>          }
>
> So far so good. Now cross-reference this to the version of src/inline.c from v2.065.0 and you will find that the function visitCallExp() does not exist. Further the function resident at the same location in the older version of the file [Expression * SliceExp::inlineScan(InlineScanState *iss){}] does not exist anywhere in the newer version (neither is any of a number of inlineScan() related implementations surrounding it). As such, it is not possible to pick this fix in preparation for v2.065.1.
>
> What is the proper resolution for this problem in the context of the 2.065 branch?
>
>
>
> _______________________________________________
> dmd-beta mailing list
> dmd-beta@puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-beta

_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta