Skip to content

Commit

Permalink
Refactor SymbolTable and AST parent logic (#732)
Browse files Browse the repository at this point in the history 
* Basic symbol table provider logic

* Better SymbolTable naming

* Fix aggregate namespace issue.

* fix m ref in symboltable

* Fix namespace name not being walked

* tweak name of link function

* Restore some missing deprecated AST props.

* Fix compile error
  • Loading branch information
@TwitchBronBron
TwitchBronBron committed Nov 1, 2022
1 parent 4d850ab commit 1feb2e8
Show file tree
Hide file tree
Showing 15 changed files with 341 additions and 161 deletions.
3 changes: 2 additions & 1 deletion .vscode/settings.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@
"editor.insertSpaces": true,
"typescript.format.insertSpaceAfterFunctionKeywordForAnonymousFunctions": true,
"typescript.format.insertSpaceAfterOpeningAndBeforeClosingNonemptyBrackets": false,
"files.trimTrailingWhitespace": true
"files.trimTrailingWhitespace": true,
"typescript.tsdk": "node_modules\\typescript\\lib"
}
2 changes: 1 addition & 1 deletion src/Scope.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('Scope', () => {
program.validate();
const scope = program.getScopesForFile('source/alpha.bs')[0];
scope.linkSymbolTable();
const symbolTable = file.parser.references.namespaceStatements[1].getSymbolTable();
const symbolTable = file.parser.references.namespaceStatements[1].body.getSymbolTable();
//the symbol table should contain the relative names for all items in this namespace across the entire scope
expect(
symbolTable.hasSymbol('Beta')
Expand Down
22 changes: 13 additions & 9 deletions src/Scope.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ export class Scope {
enumStatements: new Map(),
constStatements: new Map(),
statements: [],
symbolTable: new SymbolTable(this.symbolTable, `Namespace ${loopName}`)
symbolTable: new SymbolTable(`Namespace Aggregate: '${loopName}'`, () => this.symbolTable)
});
}
}
Expand All @@ -593,7 +593,7 @@ export class Scope {
}
// Merges all the symbol tables of the namespace statements into the new symbol table created above.
// Set those symbol tables to have this new merged table as a parent
ns.symbolTable.mergeSymbolTable(namespaceStatement.getSymbolTable());
ns.symbolTable.mergeSymbolTable(namespaceStatement.body.getSymbolTable());
}

//associate child namespaces with their parents
Expand Down Expand Up @@ -719,9 +719,9 @@ export class Scope {
this.cache.clear();
}

public get symbolTable() {
public get symbolTable(): SymbolTable {
return this.cache.getOrAdd('symbolTable', () => {
const result = new SymbolTable(this.getParentScope()?.symbolTable, `Scope ${this.name}`);
const result = new SymbolTable(`Scope: '${this.name}'`, () => this.getParentScope()?.symbolTable);
for (let file of this.getOwnFiles()) {
if (isBrsFile(file)) {
result.mergeSymbolTable(file.parser?.symbolTable);
Expand All @@ -739,13 +739,14 @@ export class Scope {
public linkSymbolTable() {
for (const file of this.getAllFiles()) {
if (isBrsFile(file)) {
file.parser.symbolTable.pushParent(this.symbolTable);
file.parser.symbolTable.pushParentProvider(() => this.symbolTable);

//link each NamespaceStatement's SymbolTable with the aggregate NamespaceLookup SymbolTable
for (const namespace of file.parser.references.namespaceStatements) {
const namespaceNameLower = namespace.getName(ParseMode.BrighterScript).toLowerCase();
const namespaceSymbolTable = this.namespaceLookup.get(namespaceNameLower).symbolTable;
namespace.getSymbolTable().pushParent(namespaceSymbolTable);
namespace.getSymbolTable().addSibling(
this.namespaceLookup.get(namespaceNameLower).symbolTable
);
}
}
}
Expand All @@ -754,10 +755,13 @@ export class Scope {
public unlinkSymbolTable() {
for (let file of this.getOwnFiles()) {
if (isBrsFile(file)) {
file.parser?.symbolTable.popParent();
file.parser?.symbolTable.popParentProvider();

for (const namespace of file.parser.references.namespaceStatements) {
namespace.getSymbolTable().popParent();
const namespaceNameLower = namespace.getName(ParseMode.BrighterScript).toLowerCase();
namespace.getSymbolTable().removeSibling(
this.namespaceLookup.get(namespaceNameLower).symbolTable
);
}
}
}
Expand Down
41 changes: 30 additions & 11 deletions src/SymbolTable.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,45 @@ import { SymbolTable } from './SymbolTable';
import { expect } from './chai-config.spec';
import { StringType } from './types/StringType';
import { IntegerType } from './types/IntegerType';

import { BooleanType } from './types/BooleanType';

describe('SymbolTable', () => {
let parent: SymbolTable;

beforeEach(() => {
parent = new SymbolTable('Parent');
});

it('is case insensitive', () => {
const st = new SymbolTable();
const st = new SymbolTable('Child');
st.addSymbol('foo', null, new StringType());
expect(st.getSymbol('FOO').length).eq(1);
expect(st.getSymbol('FOO')[0].type.toString()).eq('string');
});

it('stores all previous symbols', () => {
const st = new SymbolTable();
const st = new SymbolTable('Child');
st.addSymbol('foo', null, new StringType());
st.addSymbol('foo', null, new IntegerType());
expect(st.getSymbol('FOO').length).eq(2);
});


it('reads from parent symbol table if not found in current', () => {
const parent = new SymbolTable();
const st = new SymbolTable(parent);
const st = new SymbolTable('Child', () => parent);
parent.addSymbol('foo', null, new StringType());
expect(st.getSymbol('foo')[0].type.toString()).eq('string');
});

it('reads from current table if it exists', () => {
const parent = new SymbolTable();
const st = new SymbolTable(parent);
const st = new SymbolTable('Child', () => parent);
parent.addSymbol('foo', null, new StringType());
st.addSymbol('foo', null, new IntegerType());
expect(st.getSymbol('foo')[0].type.toString()).eq('integer');
});

it('correct checks if a symbol is in the table using hasSymbol', () => {
const parent = new SymbolTable();
const child = new SymbolTable(parent);
const child = new SymbolTable('Child', () => parent);
parent.addSymbol('foo', null, new StringType());
child.addSymbol('bar', null, new IntegerType());
expect(parent.hasSymbol('foo')).to.be.true;
Expand All @@ -50,12 +53,28 @@ describe('SymbolTable', () => {
describe('mergeSymbolTable', () => {

it('adds each symbol to the table', () => {
const st = new SymbolTable();
const st = new SymbolTable('Child');
st.addSymbol('foo', null, new StringType());
const otherTable = new SymbolTable();
const otherTable = new SymbolTable('OtherTable');
otherTable.addSymbol('bar', null, new IntegerType());
otherTable.addSymbol('foo', null, new IntegerType());
st.mergeSymbolTable(otherTable);
});
});

it('searches siblings before parents', () => {
parent.addSymbol('alpha', null, new StringType());

const child = new SymbolTable('Child', () => parent);

const sibling = new SymbolTable('Sibling');
child.addSibling(sibling);
sibling.addSymbol('alpha', null, new BooleanType());

expect(
child.getSymbol('alpha').map(x => x.type.toTypeString())
).to.eql([
'boolean'
]);
});
});
79 changes: 53 additions & 26 deletions src/SymbolTable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import type { BscType } from './types/BscType';
*/
export class SymbolTable {
constructor(
parent?: SymbolTable | undefined,
public name = ''
public name: string,
parentProvider?: SymbolTableProvider
) {
this.pushParent(parent);
if (parentProvider) {
this.pushParentProvider(parentProvider);
}
}

/**
Expand All @@ -19,28 +21,43 @@ export class SymbolTable {
*/
private symbolMap = new Map<string, BscSymbol[]>();

public get parent() {
return this.parentStack[0];
private parentProviders = [] as SymbolTableProvider[];

/**
* Push a function that will provide a parent SymbolTable when requested
*/
public pushParentProvider(provider: SymbolTableProvider) {
this.parentProviders.push(provider);
}

/**
* Sets the parent table for lookups. There can only be one parent at a time, but sometimes you
* want to temporarily change the parent, and then restore it later. This allows that.
*
* @param [parent]
* Pop the current parentProvider
*/
private parentStack: SymbolTable[] = [];
public popParentProvider() {
this.parentProviders.pop();
}

public pushParent(parent?: SymbolTable) {
this.parentStack.unshift(parent);
return parent;
/**
* The parent SymbolTable (if there is one)
*/
public get parent() {
return this.parentProviders[this.parentProviders.length - 1]?.();
}

private siblings = new Set<SymbolTable>();

/**
* Remove the current parent, restoring the previous parent (if there was one)
* Add a sibling symbol table (which will be inspected first before walking upward to the parent
*/
public popParent() {
return this.parentStack.shift();
public addSibling(sibling: SymbolTable) {
this.siblings.add(sibling);
}

/**
* Remove a sibling symbol table
*/
public removeSibling(sibling: SymbolTable) {
this.siblings.delete(sibling);
}

/**
Expand All @@ -52,12 +69,7 @@ export class SymbolTable {
* @returns true if this symbol is in the symbol table
*/
hasSymbol(name: string, searchParent = true): boolean {
const key = name.toLowerCase();
let result = this.symbolMap.has(key);
if (!result && searchParent) {
result = !!this.parent?.hasSymbol(key);
}
return result;
return !!this.getSymbol(name, searchParent);
}

/**
Expand All @@ -70,11 +82,21 @@ export class SymbolTable {
*/
getSymbol(name: string, searchParent = true): BscSymbol[] {
const key = name.toLowerCase();
let result = this.symbolMap.get(key);
if (!result && searchParent) {
result = this.parent?.getSymbol(key);
let result: BscSymbol[];
// look in our map first
if ((result = this.symbolMap.get(key))) {
return result;
}
//look through any sibling maps next
for (let sibling of this.siblings) {
if ((result = sibling.symbolMap.get(key))) {
return result;
}
}
// ask our parent for a symbol
if (searchParent && (result = this.parent?.getSymbol(key))) {
return result;
}
return result;
}

/**
Expand Down Expand Up @@ -131,3 +153,8 @@ export interface BscSymbol {
range: Range;
type: BscType;
}

/**
* A function that returns a symbol table.
*/
export type SymbolTableProvider = () => SymbolTable;
2 changes: 1 addition & 1 deletion src/astUtils/reflection.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('reflection', () => {
const expr = createStringLiteral('', range);
const token = createToken(TokenKind.StringLiteral, '', range);
const body = new Body([]);
const assignment = new AssignmentStatement(undefined, ident, expr, undefined);
const assignment = new AssignmentStatement(undefined, ident, expr);
const block = new Block([], range);
const expression = new ExpressionStatement(expr);
const comment = new CommentStatement([token]);
Expand Down
Loading

0 comments on commit 1feb2e8

Please sign in to comment.