Skip to content

Remove StackInterpreter interruptPending instance variable#793

Merged
guillep merged 1 commit intopharo-10from
stack-interpreter-interrump-pending
May 2, 2024
Merged

Remove StackInterpreter interruptPending instance variable#793
guillep merged 1 commit intopharo-10from
stack-interpreter-interrump-pending

Conversation

@jordanmontt
Copy link
Member

The instance variable interruptPending from the StackInterpreter class is only used in this method and it's never initialized (see the file changed from this PR). When this code gets compiled to C we get the following.

/* StackInterpreter>>#checkForEventsMayContextSwitch: */
static sqInt NoDbgRegParms
checkForEventsMayContextSwitch(sqInt mayContextSwitch)

sqInt interruptPending;

/* more code ... */

	if (GIV(interruptPending)) {
		GIV(interruptPending) = 0;
		
		
		sema = unsignedLongAt((GIV(specialObjectsOop) + BaseHeaderSize) + ((sqInt) (((usqInt) TheInterruptSemaphore ) << (shiftForWord())) ));
		if ((sema != GIV(nilObj)) && (synchronousSignal(sema))) {
			switched = 1;
		}
	}
	
/* more code ... */

The variable get declared like this sqInt interruptPending; but it's never initialized. So, the conditional if (GIV(interruptPending)) will never be true as the variable was never initialized.

As the interruptPending variable is only used in that method and never initialized, I propose to delete that code snipped as it's never reached and it will never be true.

@jordanmontt jordanmontt requested a review from guillep May 2, 2024 10:18
Copy link
Member

@guillep guillep left a comment

Choose a reason for hiding this comment

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

Yes!

@guillep guillep force-pushed the stack-interpreter-interrump-pending branch from 365f612 to 5f871b1 Compare May 2, 2024 12:42
@guillep guillep merged commit 48ddfee into pharo-10 May 2, 2024
@jordanmontt jordanmontt deleted the stack-interpreter-interrump-pending branch May 2, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants