Skip to content

Commit

Permalink
feat: add endless recursion as an impurity reason (#788)
Browse files Browse the repository at this point in the history
### Summary of Changes

Endless recursion is now included in the inferred impurity reasons of an
expression/callable.
  • Loading branch information
lars-reimann authored Nov 21, 2023
1 parent 6f45dc4 commit 98acdde
Show file tree
Hide file tree
Showing 29 changed files with 237 additions and 305 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,13 @@ export class SafeDsCallGraphComputer {
const call = this.createSyntheticCallForCall(node, substitutions);
return this.getCallGraphWithRecursionCheck(call, []);
} else {
const children = this.getExecutedCallsInCallable(node, substitutions).map((it) => {
return this.getCallGraphWithRecursionCheck(it, []);
});
return new CallGraph(
node,
this.getExecutedCallsInCallable(node, substitutions).map((it) => {
return this.getCallGraphWithRecursionCheck(it, []);
}),
children,
children.some((it) => it.isRecursive),
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ export class SafeDsPythonGenerator {
}

private generateBlock(block: SdsBlock, frame: GenerationInfoFrame): CompositeGeneratorNode {
let statements = getStatements(block).filter((stmt) => this.hasStatementEffect(stmt));
let statements = getStatements(block).filter((stmt) => this.purityComputer.statementDoesSomething(stmt));
if (statements.length === 0) {
return traceToNode(block)('pass');
}
Expand All @@ -368,21 +368,6 @@ export class SafeDsPythonGenerator {
})!;
}

private hasStatementEffect(statement: SdsStatement): boolean {
if (isSdsAssignment(statement)) {
const assignees = getAssignees(statement);
return (
assignees.some((value) => !isSdsWildcard(value)) ||
(statement.expression !== undefined &&
this.purityComputer.expressionHasSideEffects(statement.expression))
);
} else if (isSdsExpressionStatement(statement)) {
return this.purityComputer.expressionHasSideEffects(statement.expression);
}
/* c8 ignore next */
return false;
}

private generateStatement(statement: SdsStatement, frame: GenerationInfoFrame): CompositeGeneratorNode {
if (isSdsAssignment(statement)) {
return traceToNode(statement)(this.generateAssignment(statement, frame));
Expand Down
17 changes: 17 additions & 0 deletions packages/safe-ds-lang/src/language/purity/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,23 @@ export class PotentiallyImpureParameterCall extends ImpurityReason {
}
}

/**
* A function contains a call that leads to endless recursion.
*/
class EndlessRecursionClass extends ImpurityReason {
override isSideEffect = true;

override equals(other: unknown): boolean {
return other instanceof EndlessRecursionClass;
}

override toString(): string {
return 'Endless recursion';
}
}

export const EndlessRecursion = new EndlessRecursionClass();

/**
* A function is impure due to some reason that is not covered by the other impurity reasons.
*/
Expand Down
102 changes: 80 additions & 22 deletions packages/safe-ds-lang/src/language/purity/safe-ds-purity-computer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,30 @@ import { isEmpty } from '../../helpers/collectionUtils.js';
import type { SafeDsCallGraphComputer } from '../flow/safe-ds-call-graph-computer.js';
import type { SafeDsServices } from '../safe-ds-module.js';
import {
EndlessRecursion,
FileRead,
FileWrite,
type ImpurityReason,
OtherImpurityReason,
PotentiallyImpureParameterCall,
} from './model.js';
import {
isSdsAssignment,
isSdsExpressionStatement,
isSdsFunction,
isSdsLambda,
isSdsWildcard,
SdsCall,
SdsCallable,
SdsExpression,
SdsFunction,
SdsParameter,
SdsStatement,
} from '../generated/ast.js';
import { EvaluatedEnumVariant, ParameterSubstitutions, StringConstant } from '../partialEvaluation/model.js';
import { SafeDsAnnotations } from '../builtins/safe-ds-annotations.js';
import { SafeDsImpurityReasons } from '../builtins/safe-ds-enums.js';
import { getParameters } from '../helpers/nodeProperties.js';
import { getAssignees, getParameters } from '../helpers/nodeProperties.js';
import { isContainedInOrEqual } from '../helpers/astUtils.js';

export class SafeDsPurityComputer {
Expand Down Expand Up @@ -63,7 +68,7 @@ export class SafeDsPurityComputer {
* The parameter substitutions to use. These are **not** the argument of a call, but the values of the parameters
* of any containing callables, i.e. the context of the node.
*/
isPureCallable(node: SdsCallable, substitutions = NO_SUBSTITUTIONS): boolean {
isPureCallable(node: SdsCallable | undefined, substitutions = NO_SUBSTITUTIONS): boolean {
return isEmpty(this.getImpurityReasonsForCallable(node, substitutions));
}

Expand All @@ -77,7 +82,7 @@ export class SafeDsPurityComputer {
* The parameter substitutions to use. These are **not** the argument of a call, but the values of the parameters
* of any containing callables, i.e. the context of the node.
*/
isPureExpression(node: SdsExpression, substitutions = NO_SUBSTITUTIONS): boolean {
isPureExpression(node: SdsExpression | undefined, substitutions = NO_SUBSTITUTIONS): boolean {
return isEmpty(this.getImpurityReasonsForExpression(node, substitutions));
}

Expand All @@ -91,7 +96,7 @@ export class SafeDsPurityComputer {
* The parameter substitutions to use. These are **not** the argument of a call, but the values of the parameters
* of any containing callables, i.e. the context of the node.
*/
callableHasSideEffects(node: SdsCallable, substitutions = NO_SUBSTITUTIONS): boolean {
callableHasSideEffects(node: SdsCallable | undefined, substitutions = NO_SUBSTITUTIONS): boolean {
return this.getImpurityReasonsForCallable(node, substitutions).some((it) => it.isSideEffect);
}

Expand All @@ -105,10 +110,37 @@ export class SafeDsPurityComputer {
* The parameter substitutions to use. These are **not** the argument of a call, but the values of the parameters
* of any containing callables, i.e. the context of the node.
*/
expressionHasSideEffects(node: SdsExpression, substitutions = NO_SUBSTITUTIONS): boolean {
expressionHasSideEffects(node: SdsExpression | undefined, substitutions = NO_SUBSTITUTIONS): boolean {
return this.getImpurityReasonsForExpression(node, substitutions).some((it) => it.isSideEffect);
}

/**
* Returns whether the given statement does something. It must either
* - create a placeholder,
* - assign to a result, or
* - call a function that has side effects.
*
* @param node
* The statement to check.
*
* @param substitutions
* The parameter substitutions to use. These are **not** the argument of a call, but the values of the parameters
* of any containing callables, i.e. the context of the node.
*/
statementDoesSomething(node: SdsStatement, substitutions = NO_SUBSTITUTIONS): boolean {
if (isSdsAssignment(node)) {
return (
!getAssignees(node).every(isSdsWildcard) ||
this.expressionHasSideEffects(node.expression, substitutions)
);
} else if (isSdsExpressionStatement(node)) {
return this.expressionHasSideEffects(node.expression, substitutions);
} else {
/* c8 ignore next 2 */
return false;
}
}

/**
* Returns the reasons why the given callable is impure.
*
Expand All @@ -119,7 +151,7 @@ export class SafeDsPurityComputer {
* The parameter substitutions to use. These are **not** the argument of a call, but the values of the parameters
* of any containing callables, i.e. the context of the node.
*/
getImpurityReasonsForCallable(node: SdsCallable, substitutions = NO_SUBSTITUTIONS): ImpurityReason[] {
getImpurityReasonsForCallable(node: SdsCallable | undefined, substitutions = NO_SUBSTITUTIONS): ImpurityReason[] {
return this.getImpurityReasons(node, substitutions);
}

Expand All @@ -133,28 +165,54 @@ export class SafeDsPurityComputer {
* The parameter substitutions to use. These are **not** the argument of a call, but the values of the parameters
* of any containing callables, i.e. the context of the node.
*/
getImpurityReasonsForExpression(node: SdsExpression, substitutions = NO_SUBSTITUTIONS): ImpurityReason[] {
getImpurityReasonsForExpression(
node: SdsExpression | undefined,
substitutions = NO_SUBSTITUTIONS,
): ImpurityReason[] {
return this.getExecutedCallsInExpression(node).flatMap((it) => this.getImpurityReasons(it, substitutions));
}

private getImpurityReasons(node: SdsCall | SdsCallable, substitutions = NO_SUBSTITUTIONS): ImpurityReason[] {
const key = this.getNodeId(node);
return this.reasonsCache.get(key, () => {
return this.callGraphComputer
.getCallGraph(node, substitutions)
.streamCalledCallables()
.flatMap((it) => {
if (isSdsFunction(it)) {
return this.getImpurityReasonsForFunction(it);
} else {
return EMPTY_STREAM;
}
})
.toArray();
private getImpurityReasons(
node: SdsCall | SdsCallable | undefined,
substitutions = NO_SUBSTITUTIONS,
): ImpurityReason[] {
if (!node) {
/* c8 ignore next 2 */
return [];
}

// Cache the result if no substitutions are given
if (isEmpty(substitutions)) {
const key = this.getNodeId(node);
return this.reasonsCache.get(key, () => {
return this.doGetImpurityReasons(node, substitutions);
});
} else {
/* c8 ignore next 2 */
return this.doGetImpurityReasons(node, substitutions);
}
}

private doGetImpurityReasons(node: SdsCall | SdsCallable, substitutions = NO_SUBSTITUTIONS): ImpurityReason[] {
const callGraph = this.callGraphComputer.getCallGraph(node, substitutions);

const recursionImpurityReason: ImpurityReason[] = [];
if (callGraph.isRecursive) {
recursionImpurityReason.push(EndlessRecursion);
}

const otherImpurityReasons = callGraph.streamCalledCallables().flatMap((it) => {
if (isSdsFunction(it)) {
return this.getImpurityReasonsForFunction(it);
} else {
return EMPTY_STREAM;
}
});

return [...recursionImpurityReason, ...otherImpurityReasons];
}

private getExecutedCallsInExpression(expression: SdsExpression): SdsCall[] {
private getExecutedCallsInExpression(expression: SdsExpression | undefined): SdsCall[] {
return this.callGraphComputer.getAllContainedCalls(expression).filter((it) => {
// Keep only calls that are not contained in a lambda inside the expression
const containingLambda = getContainerOfType(it, isSdsLambda);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,57 +1,18 @@
import {
isSdsAssignment,
isSdsExpressionStatement,
isSdsWildcard,
SdsExpression,
SdsStatement,
} from '../../../generated/ast.js';
import { SdsStatement } from '../../../generated/ast.js';
import { ValidationAcceptor } from 'langium';
import { SafeDsServices } from '../../../safe-ds-module.js';
import { getAssignees } from '../../../helpers/nodeProperties.js';

export const CODE_STATEMENT_HAS_NO_EFFECT = 'statement/has-no-effect';

export const statementMustDoSomething = (services: SafeDsServices) => {
const statementDoesSomething = statementDoesSomethingProvider(services);
const purityComputer = services.purity.PurityComputer;

return (node: SdsStatement, accept: ValidationAcceptor): void => {
if (!statementDoesSomething(node)) {
if (!purityComputer.statementDoesSomething(node)) {
accept('warning', 'This statement does nothing.', {
node,
code: CODE_STATEMENT_HAS_NO_EFFECT,
});
}
};
};

const statementDoesSomethingProvider = (services: SafeDsServices) => {
const statementExpressionDoesSomething = statementExpressionDoesSomethingProvider(services);

return (node: SdsStatement): boolean => {
if (isSdsAssignment(node)) {
return !getAssignees(node).every(isSdsWildcard) || statementExpressionDoesSomething(node.expression);
} else if (isSdsExpressionStatement(node)) {
return statementExpressionDoesSomething(node.expression);
} else {
/* c8 ignore next 2 */
return false;
}
};
};

const statementExpressionDoesSomethingProvider = (services: SafeDsServices) => {
const callGraphComputer = services.flow.CallGraphComputer;
const purityComputer = services.purity.PurityComputer;

return (node: SdsExpression | undefined): boolean => {
if (!node) {
/* c8 ignore next 2 */
return false;
}

return (
callGraphComputer.getAllContainedCalls(node).some((it) => callGraphComputer.isRecursive(it)) ||
!purityComputer.isPureExpression(node)
);
};
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { AssertionError } from 'assert';
import { AstNode, EmptyFileSystem } from 'langium';
import { clearDocuments } from 'langium/test';
import { afterEach, describe, expect, it } from 'vitest';
import { describe, expect, it } from 'vitest';
import {
isSdsAnnotation,
isSdsAttribute,
Expand All @@ -21,10 +20,6 @@ const commentProvider = services.documentation.CommentProvider;
const testComment = '/* test */';

describe('SafeDsCommentProvider', () => {
afterEach(async () => {
await clearDocuments(services);
});

const testCases: CommentProviderTest[] = [
{
testName: 'commented module member (without annotations)',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { AstNode, EmptyFileSystem, expandToString } from 'langium';
import { clearDocuments } from 'langium/test';
import { afterEach, describe, expect, it } from 'vitest';
import { describe, expect, it } from 'vitest';
import { normalizeLineBreaks } from '../../../src/helpers/stringUtils.js';
import {
isSdsAnnotation,
Expand All @@ -17,10 +16,6 @@ const documentationProvider = services.documentation.DocumentationProvider;
const testDocumentation = 'Lorem ipsum.';

describe('SafeDsDocumentationProvider', () => {
afterEach(async () => {
await clearDocuments(services);
});

const testCases: DocumentationProviderTest[] = [
{
testName: 'module member',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,15 @@
import { createSafeDsServices } from '../../../src/language/index.js';
import { clearDocuments } from 'langium/test';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { createSafeDsServicesWithBuiltins } from '../../../src/language/index.js';
import { describe, expect, it } from 'vitest';
import { NodeFileSystem } from 'langium/node';
import { createGenerationTests } from './creator.js';
import { loadDocuments } from '../../helpers/testResources.js';
import { stream } from 'langium';

const services = createSafeDsServices(NodeFileSystem).SafeDs;
const services = (await createSafeDsServicesWithBuiltins(NodeFileSystem)).SafeDs;
const pythonGenerator = services.generation.PythonGenerator;
const generationTests = createGenerationTests();

describe('generation', async () => {
beforeEach(async () => {
// Load the builtin library
await services.shared.workspace.WorkspaceManager.initializeWorkspace([]);
});

afterEach(async () => {
await clearDocuments(services);
});

it.each(await generationTests)('$testName', async (test) => {
// Test is invalid
if (test.error) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
import { afterEach, describe, it } from 'vitest';
import { describe, it } from 'vitest';
import { createSafeDsServices } from '../../../src/language/index.js';
import { AssertionError } from 'assert';
import { NodeFileSystem } from 'langium/node';
import { createGrammarTests } from './creator.js';
import { clearDocuments } from 'langium/test';
import { getSyntaxErrors } from '../../helpers/diagnostics.js';

const services = createSafeDsServices(NodeFileSystem).SafeDs;

describe('grammar', () => {
afterEach(async () => {
await clearDocuments(services);
});

it.each(createGrammarTests())('$testName', async (test) => {
// Test is invalid
if (test.error) {
Expand Down
Loading

0 comments on commit 98acdde

Please sign in to comment.