diff --git a/.github/workflows/fixme-check.yml b/.github/workflows/fixme-check.yml new file mode 100644 index 0000000000..7e34fea8ca --- /dev/null +++ b/.github/workflows/fixme-check.yml @@ -0,0 +1,21 @@ +# .github/workflows/fixme-check.yml -- check for FIXME task labels +# +# This is a GitHub CI workflow +# +# to check for FIXME and similar task labels left unresolved in the +# MPS source tree. + +name: FIXME check + +on: + # Run as part of CI checks on branch push and on merged pull request. + - push + - pull_request + - workflow_dispatch # allow manual triggering + +jobs: + check-fixme: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - run: tool/check-fixme diff --git a/code/arenavm.c b/code/arenavm.c index fbd523eb96..c3f5efea5f 100644 --- a/code/arenavm.c +++ b/code/arenavm.c @@ -183,7 +183,7 @@ static Bool VMArenaCheck(VMArena vmArena) CHECKD(Land, VMArenaSpareLand(vmArena)); CHECKL((LandSize)(VMArenaSpareLand(vmArena)) == arena->spareCommitted); - /* FIXME: Can't check VMParams */ + /* TODO: Add sig to VMParamsStruct so it can be checked. */ return TRUE; } diff --git a/code/arg.c b/code/arg.c index a1c74204d4..43fd4c9c41 100644 --- a/code/arg.c +++ b/code/arg.c @@ -65,7 +65,9 @@ Bool ArgCheckPoolDebugOptions(Arg arg) Bool ArgCheckFun(Arg arg) { - CHECKL(FUNCHECK(arg->val.addr_method)); /* FIXME: Potential pun here */ + /* TODO: Fix potential pun here on Harvard architectures where + function pointers are not compatible with other pointers. */ + CHECKL(FUNCHECK(arg->val.addr_method)); return TRUE; } diff --git a/code/buffer.c b/code/buffer.c index ed38f1075b..5cf0f7ed41 100644 --- a/code/buffer.c +++ b/code/buffer.c @@ -357,7 +357,8 @@ void BufferFinish(Buffer buffer) AVERT(Buffer, buffer); AVER(BufferIsReady(buffer)); - BufferDetach(buffer, BufferPool(buffer)); /* FIXME: Should be in BufferAbsFinish? */ + /* TODO: Consider whether this could move to BufferAbsFinish. */ + BufferDetach(buffer, BufferPool(buffer)); Method(Inst, buffer, finish)(MustBeA(Inst, buffer)); } diff --git a/code/config.h b/code/config.h index 83f985a885..02d335efec 100644 --- a/code/config.h +++ b/code/config.h @@ -10,9 +10,18 @@ * external build system (gnumake, nmake, etc.) into specific sets * of features used by MPS modules. * + * TODO: It also defines defaults for various parts of the MPS. + * Perhaps this should be split into a separate header with better + * documentation. See + * . + * * DESIGN * * . + * + * TODO: Many of the default constants defined in this file do not + * have documented justification. See GitHub issue #176 + * . */ #ifndef config_h @@ -393,7 +402,10 @@ /* Pool MVT Configuration -- see */ -/* FIXME: These numbers were lifted from mv2test and need thought. */ + +/* TODO: These numbers were lifted from mv2test and need thought. See + GitHub issue #176 + . */ #define MVT_ALIGN_DEFAULT MPS_PF_ALIGN #define MVT_MIN_SIZE_DEFAULT MPS_PF_ALIGN diff --git a/code/prmcxc.h b/code/prmcxc.h index be2b6d0a97..ba26393937 100644 --- a/code/prmcxc.h +++ b/code/prmcxc.h @@ -26,8 +26,8 @@ typedef struct MutatorContextStruct { Addr address; /* Fault address, if stopped by protection * fault; NULL if stopped by thread manager. */ THREAD_STATE_S *threadState; - /* FIXME: Might need to get the floats in case the compiler stashes - intermediate values in them. */ + /* TODO: Consider getting the floats in case the compiler stashes + intermediate values in them. Never observed. */ } MutatorContextStruct; extern void MutatorContextInitFault(MutatorContext context, Addr address, THREAD_STATE_S *threadState); diff --git a/code/splay.c b/code/splay.c index 12682f7dd4..fc7873480e 100644 --- a/code/splay.c +++ b/code/splay.c @@ -1237,7 +1237,9 @@ Bool SplayFindFirst(Tree *nodeReturn, SplayTree splay, while (!found) { Tree oldRoot, newRoot; - /* FIXME: Rename to "seen" and "not yet seen" or something. */ + /* TODO: Rename to "seen" and "not yet seen" to make it clear what + these are for, and because there's nothing particularly + temporal about them. */ oldRoot = SplayTreeRoot(splay); newRoot = TreeRight(oldRoot); diff --git a/code/vmw3.c b/code/vmw3.c index 898ca1a77a..de12e0bd2a 100644 --- a/code/vmw3.c +++ b/code/vmw3.c @@ -67,6 +67,8 @@ Size PageSize(void) typedef struct VMParamsStruct { + /* TODO: Add sig and check with AVERT in VMInit and CHECKD in + VMArenaCheck. */ Bool topDown; } VMParamsStruct, *VMParams; @@ -101,7 +103,7 @@ Res VMInit(VM vm, Size size, Size grainSize, void *params) AVER(vm != NULL); AVERT(ArenaGrainSize, grainSize); AVER(size > 0); - AVER(params != NULL); /* FIXME: Should have full AVERT? */ + AVER(params != NULL); AVER(COMPATTYPE(LPVOID, Addr)); /* .assume.lpvoid-addr */ AVER(COMPATTYPE(SIZE_T, Size)); diff --git a/example/scheme/scheme-advanced.c b/example/scheme/scheme-advanced.c index 56d2a1bdf6..cdf24590d4 100644 --- a/example/scheme/scheme-advanced.c +++ b/example/scheme/scheme-advanced.c @@ -4505,7 +4505,8 @@ int main(int argc, char *argv[]) } MPS_ARGS_END(args); if (res != MPS_RES_OK) error("Couldn't create obj format"); - /* Create a chain controlling GC strategy. FIXME: explain! */ + /* Create a chain controlling GC strategy. */ + /* TODO: Brief explanation with link to manual. */ res = mps_chain_create(&obj_chain, arena, LENGTH(obj_gen_params), diff --git a/example/scheme/scheme.c b/example/scheme/scheme.c index 7ffeb7c8ef..c5f088a312 100644 --- a/example/scheme/scheme.c +++ b/example/scheme/scheme.c @@ -4442,7 +4442,8 @@ int main(int argc, char *argv[]) } MPS_ARGS_END(args); if (res != MPS_RES_OK) error("Couldn't create obj format"); - /* Create a chain controlling GC strategy. FIXME: explain! */ + /* Create a chain controlling GC strategy. */ + /* TODO: Brief explanation with link to manual. */ res = mps_chain_create(&obj_chain, arena, LENGTH(obj_gen_params), diff --git a/tool/check-fixme b/tool/check-fixme new file mode 100755 index 0000000000..06f6ef2531 --- /dev/null +++ b/tool/check-fixme @@ -0,0 +1,84 @@ +#!/bin/sh +# tool/check-fixme -- look for FIXME task labels in the MPS source tree +# Richard Brooksby, Ravenbrook Limited, 2023-01-15 +# +# Copyright (c) 2023 Ravenbrook Limited. See end of file for license. +# +# Developers can mark code in the MPS source tree with label like +# "FIXME" and "TODO" in order to leave notes for later work. These +# labels are recognized by tools like Xcode. +# +# This script finds FIXMEs in code so that we can: +# 1. fix 'em +# 2. cause them to fail CI +# 3. prevent them leaking into master and versions +# 4. thereby making them useful for communication with self or other devs +# +# TODO: A much older convention in the MPS (predates the adoption of +# "FIXME") is to mark such things with "@@@@". We should find and fix +# those too, eventually. See GitHub issue #162 +# . + +# To avoid false positives that waste time, exclude some things that: +# - we have no control over +# - come from outside the MPS project +# - build results that are copies of other things +# - things that legit talk about FIXMEs (like this script) +# - local editor backups +find . \ + -path './tool/check-fixme' -prune -o \ + -path './.github/workflows/fixme-check.yml' -prune -o \ + -path './.git' -prune -o \ + -path './manual/tool' -prune -o \ + -path './manual/html' -prune -o \ + -path './configure' -prune -o \ + -path './autom4te.cache' -prune -o \ + -path './tool/autoconf' -prune -o \ + -name 'config.status' -prune -o \ + -name '*~' -prune -o \ + -type f -print0 | +xargs -0 sh -c '! grep -F -I -n -H -e FIXME "$@"' +# The inverted grep above ensures xargs exits with zero iff no FIXMEs +# are found. Note this is very different from ``grep -v`` which would +# find *lines* that didn't match FIXME. + +# A. REFERENCES +# +# [None] +# +# +# B. DOCUMENT HISTORY +# +# 2023-01-15 RB Created. +# +# +# C. COPYRIGHT AND LICENSE +# +# Copyright (C) 2023 Ravenbrook Limited . +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# 2. 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. +# +# +# $Id$