[OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

David T Lewis
 

when i == j, there is nothing to sort.
If we call this for the first time, there is no stack allocated by INIT(0), but stack is accessed nonetheless thru PUSH(i,j).
The best thing todo IMO is to return early.


You can view, comment on, or merge this pull request online at:

  https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473

Commit Summary

  • Fix issue #472

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW6KDA23KD46I3XYCCLQ64XXZA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IHWSHIQ", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW6KDA23KD46I3XYCCLQ64XXZA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IHWSHIQ", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

David T Lewis
 
Hi Nicolas,

we trust you and with git's merge capability conflicts are unlikely.
So why not do this on the main branch? I'm curious why you're doing this
work on a branch...

On Tue, Jan 21, 2020 at 9:42 AM Nicolas Cellier <[hidden email]>
wrote:

> when i == j, there is nothing to sort.
> If we call this for the first time, there is no stack allocated by INIT(0),
> but stack is accessed nonetheless thru PUSH(i,j).
> The best thing todo IMO is to return early.
> ------------------------------
> You can view, comment on, or merge this pull request online at:
>
> https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473
> Commit Summary
>
> - Fix issue #472
>
> File Changes
>
> - *M* platforms/Cross/plugins/Squeak3D/b3dInit.c
> <https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473/files#diff-0>
> (6)
>
> Patch Links:
>
> - https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473.patch
> - https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473.diff
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications&email_token=ADY5VUCO6Q3K7YJ6NXWSUY3Q64XXXA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IHWSHIQ>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ADY5VUDMGEOQGTCR2MOTKJ3Q64XXXANCNFSM4KJXN4FA>
> .
>


--
_,,,^..^,,,_
best, Eliot


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW2RFWOMTBZ2JEE6OODQ66YIRA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJSBIII#issuecomment-576984097", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW2RFWOMTBZ2JEE6OODQ66YIRA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJSBIII#issuecomment-576984097", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

David T Lewis
In reply to this post by David T Lewis
 

Hi Eliot,
This is a quick fix with superficial understanding of code.
So just to give a chance to review.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW4EVHFPT2BXHV7TB3LQ67IKNA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJSHWGA#issuecomment-577010456", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW4EVHFPT2BXHV7TB3LQ67IKNA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJSHWGA#issuecomment-577010456", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

Stéphane Rollandin
 
Le 22/01/2020 à 06:09, Nicolas Cellier a écrit :
> This is a quick fix with superficial understanding of code.
> So just to give a chance to review.

I would like to give it a try, but it seems the fix did not yet made its
way to the trunk (I just checked with
squeak.cog.spur_win32x86_202001231742).

Is there a 32-bit Windows WM I can download somewhere?

Best,

Stef
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

Eliot Miranda-2
 


> On Jan 23, 2020, at 3:20 PM, Stéphane Rollandin <[hidden email]> wrote:
>
> Le 22/01/2020 à 06:09, Nicolas Cellier a écrit :
>> This is a quick fix with superficial understanding of code.
>> So just to give a chance to review.
>
> I would like to give it a try, but it seems the fix did not yet made its way to the trunk (I just checked with squeak.cog.spur_win32x86_202001231742).
>
> Is there a 32-bit Windows WM I can download somewhere?

Try and build one yourself? See build.win32x86/HowToBuild

>
> Best,
>
> Stef
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

Stéphane Rollandin
 
> Try and build one yourself? See build.win32x86/HowToBuild

Well, I may just wait... :)

In fact I simply do not grok how the VM release process works. In this
case for example, around when can I expect a bundle from the
opensmalltalk repository to feature the fix?

Stef

Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

David T Lewis
In reply to this post by David T Lewis
 

@fniephaus approved this pull request.

LGTM


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW6BSNF4E44N4HZ6RIDQ7MLBHA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCS7UZHA#pullrequestreview-348081308", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW6BSNF4E44N4HZ6RIDQ7MLBHA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCS7UZHA#pullrequestreview-348081308", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

David T Lewis
In reply to this post by David T Lewis
 

I'm going to merge this as it avoids a crash. If someone doesn't agree with the fix, we can always revert it. But merging it now makes it easier for our users, and especially Stéphane, to try it out.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW6MWFETYITTPU7W4DDQ7MLCFA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ3L7OY#issuecomment-578207675", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW6MWFETYITTPU7W4DDQ7MLCFA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ3L7OY#issuecomment-578207675", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

David T Lewis
In reply to this post by David T Lewis
 

Merged #473 into Cog.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW26DH7ZPW54GY5UK6DQ7MLCXA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOWF7NWDA#event-2977880844", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/473?email_source=notifications\u0026email_token=AIJPEW26DH7ZPW54GY5UK6DQ7MLCXA5CNFSM4KJXN4FKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOWF7NWDA#event-2977880844", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

fniephaus
In reply to this post by Stéphane Rollandin
 
On Fri, Jan 24, 2020 at 5:29 PM Stéphane Rollandin
<[hidden email]> wrote:
>
>
> > Try and build one yourself? See build.win32x86/HowToBuild
>
> Well, I may just wait... :)

I've decided to merge it, so you'll be able to download a binary from
Bintray in a couple of minutes.

Fabio

>
> In fact I simply do not grok how the VM release process works. In this
> case for example, around when can I expect a bundle from the
> opensmalltalk repository to feature the fix?
>
> Stef
>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

Eliot Miranda-2
In reply to this post by Stéphane Rollandin
 
Hi Stef,

> On Jan 24, 2020, at 8:29 AM, Stéphane Rollandin <[hidden email]> wrote:
>
> 
>>
>> Try and build one yourself? See build.win32x86/HowToBuild
>
> Well, I may just wait... :)
>
> In fact I simply do not grok how the VM release process works. In this case for example, around when can I expect a bundle from the opensmalltalk repository to feature the fix?

It’s not about that.  It’s about being able to test if a vm works.  It is straight forward to build and run your own vm once you have set up the build environment.  You just run a simple script that runs make and within a couple of minutes you have your own vm and you can test if it works without having to wait for anything.  Eventually you can progress to trying to fix bugs you u cover during testing.

I understand if you don’t want to or are uninterested.  But if all that’s holding you back is anxiety that it’s really complicated I assure you that if you clone the opensmalltalk-vm repository, read and follow build.win32x86/HowToBuild that you’ll be able to build the vm within a few hours (setting up Cygwin or mingw is a big time consuming but not horribly so, and you only have to do it once).

>
> Stef
>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

Stéphane Rollandin
 
> I understand if you don’t want to or are uninterested.  But if all that’s holding you back is anxiety that it’s really complicated I assure you that if you clone the opensmalltalk-vm repository, read and follow build.win32x86/HowToBuild that you’ll be able to build the vm within a few hours (setting up Cygwin or mingw is a big time consuming but not horribly so, and you only have to do it once).
>

Actually I have the whole thing set-up from last year when we started
talking about the Squeak3D plugin.

What is really holding me back is that I spend already too much time on
my computer, doing Smalltalk stuff. The perspective of adding X hours of
C debugging is not a nice one to contemplate.

But I do not want my requests to be a burden to the VM team so if the
need arises again (and I am afraid it will since it seems that not all
edge cases are handled well by the plugin), then I will compile and test
the code myself.

Thanks for your help,

Stef
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix issue #472 (#473)

Eliot Miranda-2
 
Hi Stef,

On Fri, Jan 24, 2020 at 10:59 AM Stéphane Rollandin <[hidden email]> wrote:
 
> I understand if you don’t want to or are uninterested.  But if all that’s holding you back is anxiety that it’s really complicated I assure you that if you clone the opensmalltalk-vm repository, read and follow build.win32x86/HowToBuild that you’ll be able to build the vm within a few hours (setting up Cygwin or mingw is a big time consuming but not horribly so, and you only have to do it once).
>

Actually I have the whole thing set-up from last year when we started
talking about the Squeak3D plugin.

What is really holding me back is that I spend already too much time on
my computer, doing Smalltalk stuff. The perspective of adding X hours of
C debugging is not a nice one to contemplate.

But I do not want my requests to be a burden to the VM team so if the
need arises again (and I am afraid it will since it seems that not all
edge cases are handled well by the plugin), then I will compile and test
the code myself.

I think the useful thing for us is that you find reproducible cases and we do the debugging.  So you building a VM helps us in finding the bugs.  I don't expect you to fix the bugs, but it's been really productive with you finding them.  We've collectively fixed some major bugs and are close to having a reliable 3D renderer, which is cool.
 

Thanks for your help,

Stef


--
_,,,^..^,,,_
best, Eliot