Check for FIXME etc. during CI

.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
+# <https://docs.github.com/en/actions/using-workflows/about-workflows>
+# 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-rst:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - run: tool/check-fixme

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;
 }

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;
 }
 

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));
 }

code/config.h

@@ -393,7 +393,7 @@
 
 
 /* Pool MVT Configuration -- see <code/poolmv2.c> */
-/* FIXME: These numbers were lifted from mv2test and need thought. */
+/* TODO: These numbers were lifted from mv2test and need thought. */
 
 #define MVT_ALIGN_DEFAULT         MPS_PF_ALIGN
 #define MVT_MIN_SIZE_DEFAULT      MPS_PF_ALIGN

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);

code/splay.c

@@ -1237,7 +1237,7 @@ 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" or something. */
     oldRoot = SplayTreeRoot(splay);
     newRoot = TreeRight(oldRoot);
 

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));

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),

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),

tool/check-fixme

@@ -0,0 +1,82 @@
+#!/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.
+
+# Exclude some things that:
+# - we have no control over
+# - come from elsewhere
+# - build results that are copies of other things
+# - things that legit talk about FIXMEs (like this script)
+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 <https://www.ravenbrook.com/>.
+#
+# 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$