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

Fixed bug of internal error being thrown after a redeclaration of a variable #463

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,17 @@ To use development versions of Kipper download the
- New field:
- `KipperError.programCtx`, which contains, if `KipperError.tracebackData.errorNode` is not undefined, the program
context of the error.
- New function:
- `ensureScopeDeclarationAvailableIfNeeded`, which ensures that a scope declaration is available if needed. This
is used during the semantic analysis/type checking of a declaration statement, which may need the scope
declaration object during the processing.

### Changed

### Fixed

- Redeclaration bug causing an `InternalError` after calling the compiler
([#462](https://github.om/Luna-Klatzer/Kipper/issues/462)).
- Compiler argument bug in `KipperCompiler`, where `abortOnFirstError` didn't precede `recover`, meaning that instead
of an error being thrown the failed result was returned (As defined in the `recover` behaviour, which is incorrect).

Expand Down
10 changes: 7 additions & 3 deletions kipper/core/src/compiler/ast/analysable-ast-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,11 @@ export abstract class AnalysableASTNode<
// Start with the evaluation of the children
await this.semanticallyTypeCheckChildren();

// If the semantic analysis until now hasn't failed, do the semantic type checking of this node (if defined)
// If the semantic analysis until now has evaluated data, do the semantic type checking of this node (if defined)
// Per default, we will say the nodes themselves handle all errors, so we don't need to do anything here
// Note! Expressions do this differently and abort immediately all processing if one of the children failed.
// Additionally, this will also not check for 'this.hasFailed', as we still want to run type checking if possible
// even with a logic error in the code (so we only check for the semantic data)
if (this.semanticData && this.primarySemanticTypeChecking !== undefined) {
try {
await this.primarySemanticTypeChecking();
Expand Down Expand Up @@ -309,9 +311,11 @@ export abstract class AnalysableASTNode<
// Start with the evaluation of the children
await this.targetSemanticallyAnalyseChildren();

// If the semantic analysis until now hasn't failed, do the target semantic analysis of this node (if defined)
// If the semantic analysis until now has evaluated data, do the target semantic analysis of this node (if defined)
// Per default, we will say the nodes themselves handle all errors, so we don't need to do anything here
// Note! Expressions do this differently and abort immediately all processing if one of the children failed.
// Additionally, this will also not check for 'this.hasFailed', as we still want to run the target semantic
// analysis if possible even with a logic error in the code (so we only check for the semantic data)
if (this.semanticData && this.typeSemantics && this.targetSemanticAnalysis !== undefined) {
try {
await this.targetSemanticAnalysis(this);
Expand All @@ -335,7 +339,7 @@ export abstract class AnalysableASTNode<
}

// If the check for warnings function is defined, then call it
if (this.checkForWarnings && !this.hasFailed) {
if (!this.hasFailed && this.checkForWarnings) {
await this.checkForWarnings();
}
}
Expand Down
29 changes: 28 additions & 1 deletion kipper/core/src/compiler/ast/nodes/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ import type {
ParameterDeclarationTypeSemantics,
VariableDeclarationTypeSemantics,
} from "../type-data";
import { UnableToDetermineSemanticDataError, UndefinedDeclarationCtxError } from "../../../errors";
import {
MissingRequiredSemanticDataError,
UnableToDetermineSemanticDataError,
UndefinedDeclarationCtxError
} from "../../../errors";
import { getParseTreeSource } from "../../../utils";
import { CompoundStatement, Statement } from "./statements";
import { ScopeNode } from "../scope-node";
Expand Down Expand Up @@ -123,6 +127,28 @@ export abstract class Declaration<
return this.scopeDeclaration;
}

/**
* Ensures that this node has a {@link Declaration.scopeDeclaration scope declaration} available. This will be
* primarily used by declarations in their own analysis.
*
* This will throw an error if the scope declaration is not available.
*
* This is primarily used by the {@link Declaration.semanticTypeChecking} method, which often requires the scope
* declaration to be available. As such this is a helper method which ensures the control flow is correct and no
* invalid errors are thrown. (E.g. an internal error is thrown after a normal semantic analysis error).
*
* Intentionally this will also likely cause an {@link UndefinedSemanticsError} in case the {@link scopeDeclaration}
* is missing and {@link AnalysableASTNode.hasFailed hasFailed} is returning false. Since that's an automatic
* contradiction, it's better to ignore it here and let the {@link UndefinedSemanticsError} be thrown later.
* @throws {MissingRequiredSemanticDataError} If the scope declaration is not available.
* @since 0.11.0
*/
public ensureScopeDeclarationAvailableIfNeeded(): void {
if (this instanceof Declaration && this.hasFailed && this.scopeDeclaration === undefined) {
throw new MissingRequiredSemanticDataError();
}
}

/**
* Generates the typescript code for this item, and all children (if they exist).
* @since 0.8.0
Expand Down Expand Up @@ -591,6 +617,7 @@ export class VariableDeclaration extends Declaration<VariableDeclarationSemantic
// If the variable is defined, check whether the assignment is valid
if (semanticData.value) {
semanticData.value.ensureTypeSemanticallyValid(); // Ensure the assignment didn't fail
this.ensureScopeDeclarationAvailableIfNeeded(); // Ensure the scope declaration is available
this.programCtx.typeCheck(this).validVariableDefinition(this.getScopeDeclaration(), semanticData.value);
}
}
Expand Down