Skip to content

Commit

Permalink
Separate shareable tool type from server ITool (#4472)
Browse files Browse the repository at this point in the history
* Separate shareable tool type from server ITool

I realised we were using the "Tool" on both client and server, but the client only has the JSON-able data of Tool, not the interface/object Tool (now renamed to `ITool`).

* Fix tests
  • Loading branch information
mattgodbolt committed Dec 20, 2022
1 parent 8159c55 commit 6348917
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 43 deletions.
9 changes: 5 additions & 4 deletions lib/base-compiler.ts
Expand Up @@ -76,8 +76,9 @@ import {IAsmParser} from './parsers/asm-parser.interfaces';
import {LlvmPassDumpParser} from './parsers/llvm-pass-dump-parser';
import {PropertyGetter} from './properties.interfaces';
import {getToolchainPath} from './toolchain-utils';
import {Tool, ToolResult, ToolTypeKey} from './tooling/base-tool.interface';
import {ITool, ToolResult} from './tooling/base-tool.interface';
import * as utils from './utils';
import {Tool, ToolTypeKey} from '../types/tool.interfaces';

export class BaseCompiler implements ICompiler {
protected compiler: CompilerInfo & Record<string, any>; // TODO: Some missing types still present in Compiler type
Expand All @@ -96,7 +97,7 @@ export class BaseCompiler implements ICompiler {
protected llvmAst: LlvmAstParser;
protected toolchainPath: any;
public possibleArguments: CompilerArguments;
protected possibleTools: Tool[];
protected possibleTools: ITool[];
protected demanglerClass: any;
protected objdumperClass: any;
public outputFilebase: string;
Expand Down Expand Up @@ -147,7 +148,7 @@ export class BaseCompiler implements ICompiler {
this.toolchainPath = getToolchainPath(this.compiler.exe, this.compiler.options);

this.possibleArguments = new CompilerArguments(this.compiler.id);
this.possibleTools = _.values(compilerInfo.tools);
this.possibleTools = _.values(compilerInfo.tools) as ITool[];
const demanglerExe = this.compiler.demangler;
if (demanglerExe && this.compiler.demanglerType) {
this.demanglerClass = getDemanglerTypeByKey(this.compiler.demanglerType);
Expand Down Expand Up @@ -1361,7 +1362,7 @@ export class BaseCompiler implements ICompiler {
if (tools) {
for (const tool of tools) {
const matches = this.possibleTools.find(possibleTool => {
return possibleTool.getId() === tool.id && possibleTool.getType() === type;
return possibleTool.id === tool.id && possibleTool.type === type;
});

if (matches) {
Expand Down
2 changes: 1 addition & 1 deletion lib/options-handler.ts
Expand Up @@ -36,8 +36,8 @@ import {CompilerProps} from './properties';
import {PropertyGetter, PropertyValue} from './properties.interfaces';
import {Source} from './sources';
import {BaseTool, getToolTypeByKey} from './tooling';
import {ToolTypeKey} from './tooling/base-tool.interface';
import {asSafeVer, getHash, splitArguments, splitIntoArray} from './utils';
import {ToolTypeKey} from '../types/tool.interfaces';

// TODO: There is surely a better name for this type. Used both here and in the compiler finder.
export type OptionHandlerArguments = {
Expand Down
28 changes: 2 additions & 26 deletions lib/tooling/base-tool.interface.ts
Expand Up @@ -25,25 +25,7 @@
import {LanguageKey} from '../../types/languages.interfaces';
import {Library} from '../../types/libraries/libraries.interfaces';
import {ResultLine} from '../../types/resultline/resultline.interfaces';

export type ToolTypeKey = 'independent' | 'postcompilation';

export type ToolInfo = {
id: string;
name?: string;
type?: ToolTypeKey;
exe: string;
exclude: string[];
includeKey?: string;
options: string[];
args?: string;
languageId?: LanguageKey;
stdinHint?: string;
monacoStdin?: string;
icon?: string;
darkIcon?: string;
compilerLanguage: LanguageKey;
};
import {Tool, ToolInfo} from '../../types/tool.interfaces';

export type ToolEnv = {
ceProps: (key: string, defaultValue?: any) => string | boolean | number | undefined;
Expand All @@ -67,13 +49,7 @@ export type ToolResult = {
artifact?: Artifact;
};

export interface Tool {
readonly tool: ToolInfo;

getId(): string;

getType(): string;

export interface ITool extends Tool {
runTool(
compilationInfo: Record<any, any>,
inputFilepath?: string,
Expand Down
17 changes: 7 additions & 10 deletions lib/tooling/base-tool.ts
Expand Up @@ -35,31 +35,28 @@ import * as exec from '../exec';
import {logger} from '../logger';
import {parseOutput} from '../utils';

import {Tool, ToolEnv, ToolInfo, ToolResult, ToolTypeKey} from './base-tool.interface';
import {ITool, ToolEnv, ToolResult} from './base-tool.interface';
import {ToolInfo, ToolTypeKey} from '../../types/tool.interfaces';

const toolCounter = new PromClient.Counter({
name: 'tool_invocations_total',
help: 'Number of tool invocations',
labelNames: ['language', 'name'],
});

export class BaseTool implements Tool {
export class BaseTool implements ITool {
public readonly tool: ToolInfo;
private env: ToolEnv;
protected addOptionsToToolArgs = true;
public readonly id: string;
public readonly type: string;

constructor(toolInfo: ToolInfo, env: ToolEnv) {
this.tool = toolInfo;
this.env = env;
this.addOptionsToToolArgs = true;
}

getId() {
return this.tool.id;
}

getType(): ToolTypeKey {
return this.tool.type || 'independent';
this.id = toolInfo.id;
this.type = toolInfo.type || 'independent';
}

getUniqueFilePrefix() {
Expand Down
2 changes: 1 addition & 1 deletion static/panes/compiler.ts
Expand Up @@ -52,7 +52,6 @@ import {ComponentConfig, ToolViewState} from '../components.interfaces';
import {FiledataPair} from '../multifile-service';
import {LanguageLibs} from '../options.interfaces';
import {GccDumpFiltersState, GccDumpViewSelectedPass} from './gccdump-view.interfaces';
import {Tool} from '../../lib/tooling/base-tool.interface';
import {AssemblyInstructionInfo} from '../../lib/asm-docs/base';
import {PPOptions} from './pp-view.interfaces';
import {CompilationStatus} from '../compiler-service.interfaces';
Expand All @@ -64,6 +63,7 @@ import * as utils from '../utils';
import * as Sentry from '@sentry/browser';
import {editor} from 'monaco-editor';
import IEditorMouseEvent = editor.IEditorMouseEvent;
import {Tool} from '../../types/tool.interfaces';

const toolIcons = require.context('../../views/resources/logos', false, /\.(png|svg)$/);

Expand Down
4 changes: 4 additions & 0 deletions test/options-handler.js
Expand Up @@ -515,6 +515,8 @@ describe('Options handler', () => {
tools.should.deep.equal({
fake: {
faketool: {
id: 'faketool',
type: 'independent',
addOptionsToToolArgs: true,
tool: {
args: undefined,
Expand All @@ -534,6 +536,8 @@ describe('Options handler', () => {
},
},
someothertool: {
id: 'someothertool',
type: 'independent',
addOptionsToToolArgs: true,
tool: {
args: undefined,
Expand Down
2 changes: 1 addition & 1 deletion types/compiler.interfaces.ts
Expand Up @@ -23,10 +23,10 @@
// POSSIBILITY OF SUCH DAMAGE.

import {CompilerArguments} from '../lib/compiler-arguments';
import {Tool, ToolInfo} from '../lib/tooling/base-tool.interface';

import {Language} from './languages.interfaces';
import {Library} from './libraries/libraries.interfaces';
import {Tool, ToolInfo} from './tool.interfaces';

export type CompilerInfo = {
id: string;
Expand Down
50 changes: 50 additions & 0 deletions types/tool.interfaces.ts
@@ -0,0 +1,50 @@
// Copyright (c) 2022, Compiler Explorer Authors
// All rights reserved.
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are met:
//
// * Redistributions of source code must retain the above copyright notice,
// this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above copyright
// notice, this list of conditions and the following disclaimer in the
// documentation and/or other materials provided with the distribution.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
// POSSIBILITY OF SUCH DAMAGE.

import {LanguageKey} from './languages.interfaces';

export type ToolTypeKey = 'independent' | 'postcompilation';

export type ToolInfo = {
id: string;
name?: string;
type?: ToolTypeKey;
exe: string;
exclude: string[];
includeKey?: string;
options: string[];
args?: string;
languageId?: LanguageKey;
stdinHint?: string;
monacoStdin?: string;
icon?: string;
darkIcon?: string;
compilerLanguage: LanguageKey;
};

export type Tool = {
readonly tool: ToolInfo;
readonly id: string;
readonly type: string;
};

0 comments on commit 6348917

Please sign in to comment.