From 9d3096d2cc1e481c9e40ad359a10526591715d5a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc=20Poulhi=C3=A8s?= <dkm@kataplop.net>
Date: Wed, 30 Nov 2022 20:41:15 +0100
Subject: [PATCH] fix: Gcc dump types (#4369)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

While doing some cleaning, the typing of the GCC dump feature was found
incorrect. This change tries to fix most of it... But some details still need to
be fixed (FIXME notes left in the code).

Signed-off-by: Marc Poulhiès <dkm@kataplop.net>
---
 static/components.interfaces.ts         | 28 ++++---------
 static/components.ts                    | 43 +++++++------------
 static/event-map.ts                     |  6 +--
 static/panes/compiler.ts                | 39 +++++++++--------
 static/panes/gccdump-view.interfaces.ts | 56 +++++++++++++------------
 static/panes/gccdump-view.ts            | 44 +++++++++++--------
 6 files changed, 99 insertions(+), 117 deletions(-)

diff --git a/static/components.interfaces.ts b/static/components.interfaces.ts
index a69dfed1a..65ed06c11 100644
--- a/static/components.interfaces.ts
+++ b/static/components.interfaces.ts
@@ -25,6 +25,7 @@
 import {CompilerOutputOptions} from '../types/features/filters.interfaces';
 import {CfgState} from './panes/cfg-view.interfaces';
 import {LLVMOptPipelineViewState} from './panes/llvm-opt-pipeline.interfaces';
+import {GccDumpViewState} from './panes/gccdump-view.interfaces';
 export const COMPILER_COMPONENT_NAME = 'compiler';
 export const EXECUTOR_COMPONENT_NAME = 'executor';
 export const EDITOR_COMPONENT_NAME = 'codeEditor';
@@ -153,27 +154,12 @@ export type PopulatedAstViewState = StateWithId &
     };
 
 export type EmptyGccDumpViewState = EmptyState;
-export type GccDumpOptions =
-    | 'treeDump'
-    | 'rtlDump'
-    | 'ipaDump'
-    | 'addressOption'
-    | 'slimOption'
-    | 'rawOption'
-    | 'detailsOption'
-    | 'statsOption'
-    | 'blocksOption'
-    | 'vopsOption'
-    | 'linenoOption'
-    | 'uidOption'
-    | 'allOption'
-    | 'selectedPass';
-export type PopulatedGccDumpViewState = {
-    _compilerid: string;
-    _compilerName: string;
-    _editorid: number;
-    _treeid: number;
-} & (Record<GccDumpOptions, unknown> | EmptyState);
+export type PopulatedGccDumpViewState = StateWithId &
+    GccDumpViewState & {
+        compilerName: string;
+        editorid: number;
+        treeid: number;
+    };
 
 export type EmptyCfgViewState = EmptyState;
 export type PopulatedCfgViewState = StateWithId &
diff --git a/static/components.ts b/static/components.ts
index 85cabb9da..88015096c 100644
--- a/static/components.ts
+++ b/static/components.ts
@@ -23,6 +23,8 @@
 // POSSIBILITY OF SUCH DAMAGE.
 
 import {ParseFiltersAndOutputOptions} from '../types/features/filters.interfaces';
+import {GccDumpViewState} from './panes/gccdump-view.interfaces';
+
 import {
     EmptyCompilerState,
     ComponentConfig,
@@ -50,7 +52,6 @@ import {
     PopulatedAstViewState,
     EmptyGccDumpViewState,
     PopulatedGccDumpViewState,
-    GccDumpOptions,
     EmptyCfgViewState,
     PopulatedCfgViewState,
     PopulatedConformanceViewState,
@@ -473,41 +474,25 @@ export function getGccDumpView(): ComponentConfig<EmptyGccDumpViewState> {
 
 /** Get a gcc dump view with the given configuration. */
 export function getGccDumpViewWith(
-    id: string,
+    id: number,
     compilerName: string,
     editorid: number,
     treeid: number,
-    gccDumpOutput?: Record<GccDumpOptions, unknown>
+    gccDumpOutput: GccDumpViewState
 ): ComponentConfig<PopulatedGccDumpViewState> {
-    // TODO: remove any
-    const ret: any = {
-        _compilerid: id,
-        _compilerName: compilerName,
-        _editorid: editorid,
-        _treeid: treeid,
-    };
-
-    if (gccDumpOutput) {
-        ret.treeDump = gccDumpOutput.treeDump;
-        ret.rtlDump = gccDumpOutput.rtlDump;
-        ret.ipaDump = gccDumpOutput.ipaDump;
-        ret.addressOption = gccDumpOutput.addressOption;
-        ret.slimOption = gccDumpOutput.slimOption;
-        ret.rawOption = gccDumpOutput.rawOption;
-        ret.detailsOption = gccDumpOutput.detailsOption;
-        ret.statsOption = gccDumpOutput.statsOption;
-        ret.blocksOption = gccDumpOutput.blocksOption;
-        ret.vopsOption = gccDumpOutput.vopsOption;
-        ret.linenoOption = gccDumpOutput.linenoOption;
-        ret.uidOption = gccDumpOutput.uidOption;
-        ret.allOption = gccDumpOutput.allOption;
-        ret.selectedPass = gccDumpOutput.selectedPass;
-    }
-
     return {
         type: 'component',
         componentName: GCC_DUMP_VIEW_COMPONENT_NAME,
-        componentState: ret,
+        componentState: {
+            // PopulatedGccDumpViewState
+            id,
+            compilerName,
+            editorid,
+            treeid,
+
+            // & GccDumpFiltersState
+            ...gccDumpOutput,
+        },
     };
 }
 
diff --git a/static/event-map.ts b/static/event-map.ts
index 5fb63421e..57bc5d58f 100644
--- a/static/event-map.ts
+++ b/static/event-map.ts
@@ -28,7 +28,7 @@ import {MessageWithLocation} from '../types/resultline/resultline.interfaces';
 import {SiteSettings} from './settings';
 import {Theme} from './themes';
 import {PPOptions} from './panes/pp-view.interfaces';
-import {GccSelectedPass} from './panes/gccdump-view.interfaces';
+import {GccDumpFiltersState, GccDumpViewSelectedPass} from './panes/gccdump-view.interfaces';
 import {Motd} from './motd.interfaces';
 import {CompilerInfo} from '../types/compiler.interfaces';
 import {CompilationResult} from '../types/compilation/compilation.interfaces';
@@ -92,8 +92,8 @@ export type EventMap = {
     findExecutors: () => void;
     flagsViewClosed: (compilerId: number, options: string) => void;
     flagsViewOpened: (compilerId: number) => void;
-    gccDumpFiltersChanged: (compilerId: number, filters: CompilerOutputOptions, recompile: boolean) => void;
-    gccDumpPassSelected: (compilerId: number, pass: GccSelectedPass, recompile: boolean) => void;
+    gccDumpFiltersChanged: (compilerId: number, state: GccDumpFiltersState, recompile: boolean) => void;
+    gccDumpPassSelected: (compilerId: number, pass: GccDumpViewSelectedPass, recompile: boolean) => void;
     gccDumpUIInit: (compilerId: number) => void;
     gccDumpViewClosed: (compilerId: number) => void;
     gccDumpViewOpened: (compilerId: number) => void;
diff --git a/static/panes/compiler.ts b/static/panes/compiler.ts
index 0ef070db9..28aaa1c68 100644
--- a/static/panes/compiler.ts
+++ b/static/panes/compiler.ts
@@ -51,8 +51,7 @@ import {CompilerState} from './compiler.interfaces';
 import {ComponentConfig, ToolViewState} from '../components.interfaces';
 import {FiledataPair} from '../multifile-service';
 import {LanguageLibs} from '../options.interfaces';
-import {GccSelectedPass} from './gccdump-view.interfaces';
-import {CompilerOutputOptions} from '../../types/features/filters.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';
@@ -128,7 +127,7 @@ type CompileRequestOptions = {
         produceAst: boolean;
         produceGccDump: {
             opened: boolean;
-            pass?: GccSelectedPass;
+            pass?: GccDumpViewSelectedPass;
             treeDump?: boolean;
             rtlDump?: boolean;
             ipaDump?: boolean;
@@ -295,7 +294,7 @@ export class Compiler extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Co
     private irViewOpen: boolean;
     private llvmOptPipelineViewOpen: boolean;
     private gccDumpViewOpen: boolean;
-    private gccDumpPassSelected?: GccSelectedPass;
+    private gccDumpPassSelected?: GccDumpViewSelectedPass;
     private treeDumpEnabled?: boolean;
     private rtlDumpEnabled?: boolean;
     private ipaDumpEnabled?: boolean;
@@ -627,7 +626,7 @@ export class Compiler extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Co
 
         const createGccDumpView = () => {
             return Components.getGccDumpViewWith(
-                this.id as unknown as string,
+                this.id,
                 this.getCompilerName(),
                 this.sourceEditorId ?? 0,
                 this.sourceTreeId ?? 0,
@@ -2155,24 +2154,24 @@ export class Compiler extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Co
 
     onGccDumpFiltersChanged(
         id: number,
-        filters: CompilerOutputOptions & Record<string, boolean | undefined>,
+        dumpOpts: GccDumpFiltersState,
         reqCompile: boolean
     ): void {
         if (this.id === id) {
-            this.treeDumpEnabled = filters.treeDump !== false;
-            this.rtlDumpEnabled = filters.rtlDump !== false;
-            this.ipaDumpEnabled = filters.ipaDump !== false;
+            this.treeDumpEnabled = dumpOpts.treeDump;
+            this.rtlDumpEnabled = dumpOpts.rtlDump;
+            this.ipaDumpEnabled = dumpOpts.ipaDump;
             this.dumpFlags = {
-                address: filters.addressOption !== false,
-                slim: filters.slimOption !== false,
-                raw: filters.rawOption !== false,
-                details: filters.detailsOption !== false,
-                stats: filters.statsOption !== false,
-                blocks: filters.blocksOption !== false,
-                vops: filters.vopsOption !== false,
-                lineno: filters.linenoOption !== false,
-                uid: filters.uidOption !== false,
-                all: filters.allOption !== false,
+                address: dumpOpts.addressOption,
+                slim: dumpOpts.slimOption,
+                raw: dumpOpts.rawOption,
+                details: dumpOpts.detailsOption,
+                stats: dumpOpts.statsOption,
+                blocks: dumpOpts.blocksOption,
+                vops: dumpOpts.vopsOption,
+                lineno: dumpOpts.linenoOption,
+                uid: dumpOpts.uidOption,
+                all: dumpOpts.allOption,
             };
 
             if (reqCompile) {
@@ -2181,7 +2180,7 @@ export class Compiler extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Co
         }
     }
 
-    onGccDumpPassSelected(id: number, passObject?: GccSelectedPass, reqCompile?: boolean) {
+    onGccDumpPassSelected(id: number, passObject?: GccDumpViewSelectedPass, reqCompile?: boolean) {
         if (this.id === id) {
             this.gccDumpPassSelected = passObject;
 
diff --git a/static/panes/gccdump-view.interfaces.ts b/static/panes/gccdump-view.interfaces.ts
index 9afcb7b39..5fa43ad7c 100644
--- a/static/panes/gccdump-view.interfaces.ts
+++ b/static/panes/gccdump-view.interfaces.ts
@@ -22,33 +22,37 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-export interface GccDumpFilters {
-    treeDump: any;
-    rtlDump: any;
-    ipaDump: any;
-    addressOption: any;
-    slimOption: any;
-    rawOption: any;
-    detailsOption: any;
-    statsOption: any;
-    blocksOption: any;
-    vopsOption: any;
-    linenoOption: any;
-    uidOption: any;
-    allOption: any;
-}
-
-export interface GccDumpState extends GccDumpFilters {
+// Extra information used to serialize the state
+export interface GccDumpViewSelectedPass {
+    // FIXME(dkm): this type needs to be refactored.
+    // In particular, see in gccdump-view.ts:{constructor, getCurrentState}
+    // There is a mix of 'selectedPass' being a filename_prefix and a
+    // GccDumpViewSelectedPass object.
+    filename_suffix: string | null;
+    name: string | null;
+    command_prefix: string | null;
     selectedPass: string | null;
-    // legacy
-    _compilerid?: number;
-    _compilerName?: string;
-    _editorid?: number;
-    _treeid?: number;
 }
 
-export type GccSelectedPass = {
-    filename_suffix: string;
-    name: string;
-    command_prefix: string;
+// This should reflect the corresponding UI widget in gccdump.pug
+// Each optionButton should have a matching boolean here.
+export type GccDumpFiltersState = {
+    treeDump: boolean;
+    rtlDump: boolean;
+    ipaDump: boolean;
+
+    rawOption: boolean;
+    slimOption: boolean;
+    allOption: boolean;
+
+    addressOption: boolean;
+    blocksOption: boolean;
+    linenoOption: boolean;
+    detailsOption: boolean;
+    statsOption: boolean;
+    uidOption: boolean;
+    vopsOption: boolean;
 };
+
+// state = selected pass + all option flags
+export type GccDumpViewState = GccDumpFiltersState & GccDumpViewSelectedPass;
diff --git a/static/panes/gccdump-view.ts b/static/panes/gccdump-view.ts
index eb12b470f..ce25514e5 100644
--- a/static/panes/gccdump-view.ts
+++ b/static/panes/gccdump-view.ts
@@ -37,12 +37,11 @@ import {MonacoPane} from './pane';
 import {MonacoPaneState} from './pane.interfaces';
 import * as monacoConfig from '../monaco-config';
 
-import {GccDumpFilters, GccDumpState} from './gccdump-view.interfaces';
+import {GccDumpFiltersState, GccDumpViewState, GccDumpViewSelectedPass} from './gccdump-view.interfaces';
 
 import {ga} from '../analytics';
-import {CompilerOutputOptions} from '../../types/features/filters.interfaces';
 
-export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, GccDumpState> {
+export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, GccDumpViewState> {
     selectize: TomSelect;
     uiIsReady: boolean;
     filters: Toggles;
@@ -77,11 +76,7 @@ export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Gcc
     cursorSelectionThrottledFunction: ((e: any) => void) & _.Cancelable;
     selectedPass: string | null = null;
 
-    constructor(hub: Hub, container: Container, state: GccDumpState & MonacoPaneState) {
-        if (state._compilerid) state.id = state._compilerid;
-        if (state._compilerName) state.compilerName = state._compilerName;
-        if (state._editorid) state.editorid = state._editorid;
-        if (state._treeid) state.treeid = state._treeid;
+    constructor(hub: Hub, container: Container, state: GccDumpViewState & MonacoPaneState) {
         super(hub, container, state);
 
         if (state.selectedPass && typeof state.selectedPass === 'string') {
@@ -96,10 +91,13 @@ export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Gcc
             };
             const match = state.selectedPass.match(selectedPassRe);
             if (match) {
-                const selectedPassO = {
+                const selectedPassO : GccDumpViewSelectedPass = {
                     filename_suffix: match[1] + '.' + match[2],
                     name: match[2] + ' (' + passType[match[1]] + ')',
                     command_prefix: '-fdump-' + passType[match[1]] + '-' + match[2],
+
+                    // FIXME(dkm): maybe this could be avoided by better typing.
+                    selectedPass : null,
                 };
 
                 this.eventHub.emit('gccDumpPassSelected', this.compilerInfo.compilerId, selectedPassO, false);
@@ -114,7 +112,7 @@ export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Gcc
         this.eventHub.emit(
             'gccDumpFiltersChanged',
             this.compilerInfo.compilerId,
-            this.getEffectiveFilters() as CompilerOutputOptions,
+            this.getEffectiveFilters(),
             false
         );
 
@@ -126,11 +124,11 @@ export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Gcc
         this.eventHub.emit('gccDumpUIInit', this.compilerInfo.compilerId);
     }
 
-    override getInitialHTML() {
+    override getInitialHTML(): string {
         return $('#gccdump').html();
     }
 
-    override createEditor(editorRoot: HTMLElement) {
+    override createEditor(editorRoot: HTMLElement): monaco.editor.IStandaloneCodeEditor {
         return monaco.editor.create(
             editorRoot,
             monacoConfig.extendConfig({
@@ -150,7 +148,7 @@ export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Gcc
         });
     }
 
-    override registerButtons(state: GccDumpState & MonacoPaneState) {
+    override registerButtons(state: GccDumpViewState & MonacoPaneState) {
         super.registerButtons(state);
 
         const gccdump_picker = this.domRoot.find('.gccdump-pass-picker').get(0);
@@ -385,8 +383,11 @@ export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Gcc
         }
     }
 
-    getEffectiveFilters() {
-        return this.filters.get();
+    getEffectiveFilters() : GccDumpFiltersState {
+        // This cast only works if gccdump.pug and gccdump-view.interfaces are
+        // kept synchronized. See comment in gccdump-view.interfaces.ts.
+
+        return this.filters.get() as unknown as GccDumpFiltersState;
     }
 
     onFilterChange() {
@@ -397,7 +398,7 @@ export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Gcc
             this.eventHub.emit(
                 'gccDumpFiltersChanged',
                 this.compilerInfo.compilerId,
-                this.getEffectiveFilters() as unknown as CompilerOutputOptions,
+                this.getEffectiveFilters(),
                 true
             );
         }
@@ -405,12 +406,19 @@ export class GccDump extends MonacoPane<monaco.editor.IStandaloneCodeEditor, Gcc
 
     override getCurrentState() {
         const parent = super.getCurrentState();
-        const filters = this.getEffectiveFilters() as unknown as GccDumpFilters; // TODO: Validate somehow?
-        const state: MonacoPaneState & GccDumpState = {
+        const filters = this.getEffectiveFilters(); // TODO: Validate somehow?
+        const state: MonacoPaneState & GccDumpViewState = {
             // filters needs to come first, the entire state is given to the toggles and we don't want to override
             // properties such as selectedPass with obsolete values
             ...filters,
+
             selectedPass: this.selectedPass,
+
+            // See FIXME(dkm) comment in gccdump-view.interfaces.ts.
+            filename_suffix: this.selectedPass,
+            name: null,
+            command_prefix: null,
+
             ...parent,
         };
         // TODO(jeremy-rifkin)
-- 
GitLab