Tag all remaining FIXMEs with bugs. Install rule in tidy script to enforce this.

18 files changed, 82 insertions, 72 deletions

diff --git a/src/rt/memory_region.h b/src/rt/memory_region.h
index fbc439a92a6..be8689672a8 100644
--- a/src/rt/memory_region.h
+++ b/src/rt/memory_region.h
@@ -2,8 +2,8 @@
  * The Rust runtime uses memory regions to provide a primitive level of
  * memory management and isolation between tasks, and domains.
  *
- * FIXME: Implement a custom lock-free malloc / free instead of relying solely
- *       on the standard malloc / free. (#2686)
+ * FIXME (#2686): Implement a custom lock-free malloc / free instead of
+ *       relying solely on the standard malloc / free.
  */
 
 #ifndef MEMORY_REGION_H
diff --git a/src/rt/rust.cpp b/src/rt/rust.cpp
index 7a909c57348..67b3bf84938 100644
--- a/src/rt/rust.cpp
+++ b/src/rt/rust.cpp
@@ -67,8 +67,8 @@ command_line_args : public kernel_owned<command_line_args>
 };
 
 // A global that indicates whether Rust typestate claim statements should be
-// executed. Generated code will read this variable directly (I think).
-// FIXME: This belongs somewhere else (#2670)
+// executed Generated code will read this variable directly (I think).
+// FIXME (#2670): This belongs somewhere else
 int check_claims = 0;
 
 /**
diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp
index 3f754a44298..cab1b6b427c 100644
--- a/src/rt/rust_builtin.cpp
+++ b/src/rt/rust_builtin.cpp
@@ -232,8 +232,8 @@ rand_free(randctx *rctx) {
 
 /* Debug helpers strictly to verify ABI conformance.
  *
- * FIXME: move these into a testcase when the testsuite
- * understands how to have explicit C files included. (#2688)
+ * FIXME (#2665): move these into a testcase when the testsuite
+ * understands how to have explicit C files included.
  */
 
 struct quad {
@@ -288,19 +288,20 @@ debug_opaque(type_desc *t, uint8_t *front) {
     rust_task *task = rust_get_current_task();
     LOG(task, stdlib, "debug_opaque");
     debug_tydesc_helper(t);
-    // FIXME may want to actually account for alignment.  `front` may not
-    // indeed be the front byte of the passed-in argument. (#2667)
+    // FIXME (#2667) may want to actually account for alignment.
+    // `front` may not indeed be the front byte of the passed-in
+    // argument.
     for (uintptr_t i = 0; i < t->size; ++front, ++i) {
         LOG(task, stdlib, "  byte %" PRIdPTR ": 0x%" PRIx8, i, *front);
     }
 }
 
-// FIXME this no longer reflects the actual structure of boxes! (#2667)
+// FIXME (#2667) this no longer reflects the actual structure of boxes!
 struct rust_box {
     RUST_REFCOUNTED(rust_box)
 
-    // FIXME `data` could be aligned differently from the actual box body data
-    // (#2667)
+    // FIXME (#2667) `data` could be aligned differently from the actual
+    // box body data
     uint8_t data[];
 };
 
@@ -636,7 +637,7 @@ extern "C" CDECL rust_task*
 rust_new_task_in_sched(rust_sched_id id) {
     rust_task *task = rust_get_current_task();
     rust_scheduler *sched = task->kernel->get_scheduler_by_id(id);
-    // FIXME: What if we didn't get the scheduler? (#2668)
+    // FIXME (#2668): What if we didn't get the scheduler?
     return new_task_common(sched, task);
 }
 
diff --git a/src/rt/rust_cc.cpp b/src/rt/rust_cc.cpp
index e75ec46522f..ed31ddd9dad 100644
--- a/src/rt/rust_cc.cpp
+++ b/src/rt/rust_cc.cpp
@@ -79,7 +79,7 @@ class irc : public shape::data<irc,shape::ptr> {
         shape::ptr data_end = sub.end_dp = shape::ptr(data_range.second);
         while (sub.dp < data_end) {
             sub.walk_reset();
-            // FIXME: shouldn't this be 'sub.align = true;'?
+            // FIXME (#2669): shouldn't this be 'sub.align = true;'?
             align = true;
         }
     }
@@ -214,7 +214,7 @@ irc::walk_variant2(shape::tag_info &tinfo, uint32_t variant_id,
                    variant_ptr_and_end) {
     irc sub(*this, variant_ptr_and_end.first);
 
-    assert(variant_id < 256);   // FIXME: Temporary sanity check.
+    assert(variant_id < 256);   // FIXME (#2666): Temporary sanity check.
 
     const uint8_t *variant_end = variant_ptr_and_end.second;
     while (sub.sp < variant_end) {
@@ -329,7 +329,7 @@ class mark : public shape::data<mark,shape::ptr> {
             return;
 
         if (data_range.second - data_range.first > 100000)
-            abort();    // FIXME: Temporary sanity check.
+            abort();    // FIXME (#2666): Temporary sanity check.
 
         mark sub(*this, shape::ptr(data_range.first));
         shape::ptr data_end = sub.end_dp = shape::ptr(data_range.second);
@@ -454,7 +454,7 @@ mark::walk_variant2(shape::tag_info &tinfo, uint32_t variant_id,
                    variant_ptr_and_end) {
     mark sub(*this, variant_ptr_and_end.first);
 
-    assert(variant_id < 256);   // FIXME: Temporary sanity check.
+    assert(variant_id < 256);   // FIXME (#2666): Temporary sanity check.
 
     const uint8_t *variant_end = variant_ptr_and_end.second;
     while (sub.sp < variant_end) {
@@ -550,7 +550,9 @@ maybe_cc(rust_task *task) {
         return;
     }
 
-    // FIXME: Needs a snapshot.
+    // FIXME (#1498): depressingly, due to alignment bugs the whole file is
+    // disabled presently unless you're doing testing. Remove the whole thing
+    // when we transition to using a visitor for GC/CC.
 #if 0
     if (task->cc_counter++ > RUST_CC_FREQUENCY) {
         task->cc_counter = 0;
diff --git a/src/rt/rust_debug.h b/src/rt/rust_debug.h
index f0a03665f75..68f9e3d4519 100644
--- a/src/rt/rust_debug.h
+++ b/src/rt/rust_debug.h
@@ -34,7 +34,7 @@ public:
     flag(const char *in_name) : name(in_name), valid(false) {}
 
     bool operator*() {
-        // FIXME: We ought to lock this. (#2689)
+        // FIXME (#2689): We ought to lock this.
         if (!valid) {
             char *ev = getenv(name);
             value = ev && ev[0] != '\0' && ev[0] != '0';
diff --git a/src/rt/rust_kernel.cpp b/src/rt/rust_kernel.cpp
index de08436cf3f..b13b1490c0f 100644
--- a/src/rt/rust_kernel.cpp
+++ b/src/rt/rust_kernel.cpp
@@ -184,23 +184,22 @@ rust_kernel::run() {
     return rval;
 }
 
-// FIXME: Fix all these FIXMEs (#2690)
 void
 rust_kernel::fail() {
-    // FIXME: On windows we're getting "Application has requested the
-    // Runtime to terminate it in an unusual way" when trying to shutdown
-    // cleanly.
+    // FIXME (#2671): On windows we're getting "Application has
+    // requested the Runtime to terminate it in an unusual way" when
+    // trying to shutdown cleanly.
     set_exit_status(PROC_FAIL_CODE);
 #if defined(__WIN32__)
     exit(rval);
 #endif
     // Copy the list of schedulers so that we don't hold the lock while
     // running kill_all_tasks.
-    // FIXME: There's a lot that happens under kill_all_tasks, and I don't
-    // know that holding sched_lock here is ok, but we need to hold the
-    // sched lock to prevent the scheduler from being destroyed while
-    // we are using it. Probably we need to make rust_scheduler atomicly
-    // reference counted.
+    // FIXME (#2671): There's a lot that happens under kill_all_tasks,
+    // and I don't know that holding sched_lock here is ok, but we need
+    // to hold the sched lock to prevent the scheduler from being
+    // destroyed while we are using it. Probably we need to make
+    // rust_scheduler atomicly reference counted.
     std::vector<rust_scheduler*> scheds;
     {
         scoped_lock with(sched_lock);
@@ -210,9 +209,9 @@ rust_kernel::fail() {
         }
     }
 
-    // FIXME: This is not a foolproof way to kill all tasks while ensuring
-    // that no new tasks or schedulers are created in the meantime that
-    // keep the scheduler alive.
+    // FIXME (#2671): This is not a foolproof way to kill all tasks
+    // while ensuring that no new tasks or schedulers are created in the
+    // meantime that keep the scheduler alive.
     for (std::vector<rust_scheduler*>::iterator iter = scheds.begin();
          iter != scheds.end(); iter++) {
         (*iter)->kill_all_tasks();
diff --git a/src/rt/rust_log.cpp b/src/rt/rust_log.cpp
index 6e29d873ed2..5074b1f40c6 100644
--- a/src/rt/rust_log.cpp
+++ b/src/rt/rust_log.cpp
@@ -126,8 +126,8 @@ rust_log::trace_ln(rust_task *task, uint32_t level, char *message) {
         assert(!task->on_rust_stack() && "logging on rust stack");
     }
 
-    // FIXME: The scheduler and task names used to have meaning,
-    // but they are always equal to 'main' currently (#2672)
+    // FIXME (#2672): The scheduler and task names used to have meaning,
+    // but they are always equal to 'main' currently
 #if 0
 
 #if defined(__WIN32__)
@@ -233,8 +233,9 @@ void update_crate_map(const cratemap* map, log_directive* dirs,
     // First update log levels for this crate
     update_module_map(map->entries, dirs, n_dirs, n_matches);
     // Then recurse on linked crates
-    // FIXME this does double work in diamond-shaped deps. could keep
-    // a set of visited addresses, if it turns out to be actually slow (#2673)
+    // FIXME (#2673) this does double work in diamond-shaped deps. could
+    //   keep a set of visited addresses, if it turns out to be actually
+    //   slow
     for (size_t i = 0; map->children[i]; i++) {
         update_crate_map(map->children[i], dirs, n_dirs, n_matches);
     }
diff --git a/src/rt/rust_run_program.cpp b/src/rt/rust_run_program.cpp
index b089ac46934..739527b84eb 100644
--- a/src/rt/rust_run_program.cpp
+++ b/src/rt/rust_run_program.cpp
@@ -160,7 +160,7 @@ rust_run_program(const char* argv[],
     for (int fd = getdtablesize() - 1; fd >= 3; fd--) close(fd);
     if (dir) {
         int result = chdir(dir);
-        // FIXME: need error handling (#2674)
+        // FIXME (#2674): need error handling
         assert(!result && "chdir failed");
     }
 
diff --git a/src/rt/rust_sched_loop.h b/src/rt/rust_sched_loop.h
index be80a88c30d..61fd8ac05fd 100644
--- a/src/rt/rust_sched_loop.h
+++ b/src/rt/rust_sched_loop.h
@@ -88,6 +88,7 @@ public:
 
     randctx rctx;
     const char *const name; // Used for debugging
+
     // Only a pointer to 'name' is kept, so it must live as long as this
     // domain.
     rust_sched_loop(rust_scheduler *sched, int id);
diff --git a/src/rt/rust_scheduler.h b/src/rt/rust_scheduler.h
index 199c5f4bf64..74e6d6bf2bd 100644
--- a/src/rt/rust_scheduler.h
+++ b/src/rt/rust_scheduler.h
@@ -16,7 +16,7 @@ class rust_sched_launcher;
 class rust_sched_launcher_factory;
 
 class rust_scheduler : public kernel_owned<rust_scheduler> {
-    // FIXME: Make these private
+    // FIXME (#2693): Make these private
 public:
     rust_kernel *kernel;
 private:
diff --git a/src/rt/rust_shape.h b/src/rt/rust_shape.h
index 699a26aa562..cfac33008b9 100644
--- a/src/rt/rust_shape.h
+++ b/src/rt/rust_shape.h
@@ -84,7 +84,7 @@ public:
 
     template<typename T>
     inline T *alloc(size_t count = 1) {
-        // FIXME: align
+        // FIXME: align (probably won't fix before #1498)
         size_t sz = count * sizeof(T);
         T *rv = (T *)ptr;
         ptr += sz;
@@ -894,7 +894,8 @@ public:
     void walk_tag1(tag_info &tinfo);
 
     void walk_struct1(const uint8_t *end_sp) {
-        // FIXME: shouldn't we be aligning to the first element here?
+        // FIXME (probably won't fix before #1498): shouldn't we be aligning
+        // to the first element here?
         static_cast<T *>(this)->walk_struct2(end_sp);
     }
 
diff --git a/src/rt/rust_task.cpp b/src/rt/rust_task.cpp
index 19785e3a24e..adbd75f1c90 100644
--- a/src/rt/rust_task.cpp
+++ b/src/rt/rust_task.cpp
@@ -59,7 +59,8 @@ rust_task::delete_this()
     DLOG(sched_loop, task, "~rust_task %s @0x%" PRIxPTR ", refcnt=%d",
          name, (uintptr_t)this, ref_count);
 
-    // FIXME: We should do this when the task exits, not in the destructor
+    // FIXME (#2677): We should do this when the task exits, not in the
+    // destructor
     {
         scoped_lock with(supervisor_lock);
         if (supervisor) {
@@ -67,7 +68,7 @@ rust_task::delete_this()
         }
     }
 
-    /* FIXME: tighten this up, there are some more
+    /* FIXME (#2677): tighten this up, there are some more
        assertions that hold at task-lifecycle events. */
     assert(ref_count == 0); // ||
     //   (ref_count == 1 && this == sched->root_task));
@@ -114,13 +115,14 @@ cleanup_task(cleanup_args *args) {
         }
     }
 
-    // FIXME: For performance we should do the annihilator instead
-    // of the cycle collector even under normal termination, but
+    // FIXME (#2676): For performance we should do the annihilator
+    // instead of the cycle collector even under normal termination, but
     // since that would hide memory management errors (like not derefing
     // boxes), it needs to be disableable in debug builds.
     if (threw_exception) {
-        // FIXME: When the annihilator is more powerful and successfully
-        // runs resource destructors, etc. we can get rid of this cc
+        // FIXME (#2676): When the annihilator is more powerful and
+        // successfully runs resource destructors, etc. we can get rid
+        // of this cc
         cc::do_cc(task);
         annihilate_boxes(task);
     }
@@ -287,7 +289,7 @@ void
 rust_task::begin_failure(char const *expr, char const *file, size_t line) {
 
     if (expr) {
-        // FIXME: Change this message to be
+        // FIXME (#2678): Change this message to be
         // 'task failed at ...'
         LOG_ERR(this, task, "upcall fail '%s', %s:%" PRIdPTR,
                 expr, file, line);
@@ -301,7 +303,7 @@ rust_task::begin_failure(char const *expr, char const *file, size_t line) {
 #else
     die();
     conclude_failure();
-    // FIXME: Need unwinding on windows. This will end up aborting
+    // FIXME (#908): Need unwinding on windows. This will end up aborting
     sched_loop->fail();
 #endif
 }
@@ -458,7 +460,7 @@ rust_task::calloc(size_t size, const char *tag) {
 
 void
 rust_task::notify(bool success) {
-    // FIXME (1078) Do this in rust code
+    // FIXME (#1078) Do this in rust code
     if(notify_enabled) {
         rust_port *target_port =
             kernel->get_port_by_id(notify_port);
@@ -622,7 +624,7 @@ rust_task::reset_stack_limit() {
     uintptr_t sp = get_sp();
     // Have to do the rest on the C stack because it involves
     // freeing stack segments, logging, etc.
-    // FIXME: This probably doesn't need to happen on the C
+    // FIXME (#2679): This probably doesn't need to happen on the C
     // stack now
     reset_args ra = {this, sp};
     call_on_c_stack(&ra, (void*)reset_stack_limit_on_c_stack);
diff --git a/src/rt/rust_task.h b/src/rt/rust_task.h
index 43f1abd190e..2a58725c6a4 100644
--- a/src/rt/rust_task.h
+++ b/src/rt/rust_task.h
@@ -18,10 +18,11 @@
    threads at any time. This may keep the task from being destroyed even after
    the task is dead from a Rust task lifecycle perspective.
 
-   FIXME: The task and the scheduler have an over-complicated, undocumented
-   protocol for shutting down the task, hopefully without races. It would be
-   easier to reason about if other runtime objects could not access the task
-   from arbitrary threads, and didn't need to be atomically refcounted.
+   FIXME (#2696): The task and the scheduler have an over-complicated,
+   undocumented protocol for shutting down the task, hopefully without
+   races. It would be easier to reason about if other runtime objects could
+   not access the task from arbitrary threads, and didn't need to be
+   atomically refcounted.
  */
 
 #ifndef RUST_TASK_H
@@ -42,8 +43,9 @@
 
 // The amount of extra space at the end of each stack segment, available
 // to the rt, compiler and dynamic linker for running small functions
-// FIXME: We want this to be 128 but need to slim the red zone calls down,
-// disable lazy symbol relocation, and other things we haven't discovered yet
+// FIXME (#1509): We want this to be 128 but need to slim the red zone calls
+// down, disable lazy symbol relocation, and other things we haven't
+// discovered yet
 #define RZ_LINUX_32 (1024*2)
 #define RZ_LINUX_64 (1024*2)
 #define RZ_MAC_32   (1024*20)
@@ -303,7 +305,7 @@ public:
     void allow_kill();
 };
 
-// FIXME: It would be really nice to be able to get rid of this.
+// FIXME (#2697): It would be really nice to be able to get rid of this.
 inline void *operator new[](size_t size, rust_task *task, const char *tag) {
     return task->malloc(size, tag);
 }
@@ -360,9 +362,9 @@ sanitize_next_sp(uintptr_t next_sp) {
     // to the amount of stack needed for calling __morestack I've added some
     // extra bytes here.
 
-    // FIXME: On the rust stack this potentially puts is quite far into the
-    // red zone. Might want to just allocate a new rust stack every time we
-    // switch back to rust.
+    // FIXME (#2698): On the rust stack this potentially puts is quite far
+    // into the red zone. Might want to just allocate a new rust stack every
+    // time we switch back to rust.
     const uintptr_t padding = 16;
 
     return align_down(next_sp - padding);
@@ -416,7 +418,7 @@ rust_task::call_on_rust_stack(void *args, void *fn_ptr) {
 
     uintptr_t sp = sanitize_next_sp(next_rust_sp);
 
-    // FIXME(2047): There are times when this is called and needs
+    // FIXME (#2047): There are times when this is called and needs
     // to be able to throw, and we don't account for that.
     __morestack(args, fn_ptr, sp);
 
@@ -529,7 +531,7 @@ rust_task::record_stack_limit() {
 inline rust_task* rust_get_current_task() {
     uintptr_t sp_limit = get_sp_limit();
 
-    // FIXME (1226) - Because of a hack in upcall_call_shim_on_c_stack this
+    // FIXME (#1226) - Because of a hack in upcall_call_shim_on_c_stack this
     // value is sometimes inconveniently set to 0, so we can't use this
     // method of retreiving the task pointer and need to fall back to TLS.
     if (sp_limit == 0)
diff --git a/src/rt/rust_upcall.cpp b/src/rt/rust_upcall.cpp
index 986bc713213..940cf3aa01a 100644
--- a/src/rt/rust_upcall.cpp
+++ b/src/rt/rust_upcall.cpp
@@ -49,7 +49,7 @@ extern "C" CDECL void
 upcall_call_shim_on_c_stack(void *args, void *fn_ptr) {
     rust_task *task = rust_get_current_task();
 
-    // FIXME (1226) - The shim functions generated by rustc contain the
+    // FIXME (#1226) - The shim functions generated by rustc contain the
     // morestack prologue, so we need to let them know they have enough
     // stack.
     record_sp_limit(0);
@@ -72,9 +72,9 @@ extern "C" CDECL void
 upcall_call_shim_on_rust_stack(void *args, void *fn_ptr) {
     rust_task *task = rust_get_current_task();
 
-    // FIXME: Because of the hack in the other function that disables the
-    // stack limit when entering the C stack, here we restore the stack limit
-    // again.
+    // FIXME (#2680): Because of the hack in the other function that disables
+    // the stack limit when entering the C stack, here we restore the stack
+    // limit again.
     task->record_stack_limit();
 
     try {
@@ -86,7 +86,7 @@ upcall_call_shim_on_rust_stack(void *args, void *fn_ptr) {
         assert(false && "Rust task failed after reentering the Rust stack");
     }
 
-    // FIXME: As above
+    // FIXME (#2680): As above
     record_sp_limit(0);
 }
 
@@ -177,7 +177,7 @@ upcall_exchange_malloc(type_desc *td, uintptr_t size) {
     return args.retval;
 }
 
-// FIXME: remove after snapshot (6/21/12)
+// FIXME (#2681): remove after snapshot (6/21/12)
 extern "C" CDECL uintptr_t
 upcall_exchange_malloc_dyn(type_desc *td, uintptr_t size) {
     rust_task *task = rust_get_current_task();
@@ -246,7 +246,7 @@ upcall_malloc(type_desc *td, uintptr_t size) {
     return args.retval;
 }
 
-// FIXME: remove after snapshot (6/21/12)
+// FIXME (#2681): remove after snapshot (6/21/12)
 extern "C" CDECL uintptr_t
 upcall_malloc_dyn(type_desc *td, uintptr_t size) {
     rust_task *task = rust_get_current_task();
diff --git a/src/rt/rust_util.h b/src/rt/rust_util.h
index 2040a641c47..247f253fd9f 100644
--- a/src/rt/rust_util.h
+++ b/src/rt/rust_util.h
@@ -111,7 +111,7 @@ make_str_vec(rust_kernel* kernel, size_t nstrs, char **strs) {
 
 inline size_t get_box_size(size_t body_size, size_t body_align) {
     size_t header_size = sizeof(rust_opaque_box);
-    // FIXME: This alignment calculation is suspicious. Is it right?
+    // FIXME (#2699): This alignment calculation is suspicious. Is it right?
     size_t total_size = align_to(header_size, body_align) + body_size;
     return total_size;
 }
diff --git a/src/rt/sync/lock_and_signal.cpp b/src/rt/sync/lock_and_signal.cpp
index ccc762c2cc9..9558aaa7c06 100644
--- a/src/rt/sync/lock_and_signal.cpp
+++ b/src/rt/sync/lock_and_signal.cpp
@@ -9,7 +9,8 @@
  * if you're using a pthreads cvar+mutex pair.
  */
 
-// FIXME: This is not a portable way of specifying an invalid pthread_t
+// FIXME (#2683): This is not a portable way of specifying an invalid
+// pthread_t
 #define INVALID_THREAD 0
 
 
diff --git a/src/rt/sync/lock_free_queue.h b/src/rt/sync/lock_free_queue.h
index 3ee15d5d3a6..ed11b1aa321 100644
--- a/src/rt/sync/lock_free_queue.h
+++ b/src/rt/sync/lock_free_queue.h
@@ -88,8 +88,8 @@ class lock_free_queue {
         pointer_t *oldValue,
         pointer_t newValue) {
 
-        // FIXME this is requiring us to pass -fno-strict-aliasing to GCC
-        // (possibly there are other, similar problems)
+        // FIXME (#2701) this is requiring us to pass -fno-strict-aliasing
+        // to GCC (possibly there are other, similar problems)
         if (sync::compare_and_swap(
                 (uint64_t*) address,
                 *(uint64_t*) oldValue,
diff --git a/src/rt/sync/timer.cpp b/src/rt/sync/timer.cpp
index 28ee6da2b62..0204517d512 100644
--- a/src/rt/sync/timer.cpp
+++ b/src/rt/sync/timer.cpp
@@ -10,7 +10,7 @@ uint64_t ns_per_s = 1000000000LL;
 timer::timer() {
 #if __WIN32__
     _ticks_per_s = 0LL;
-    // FIXME: assert this works or have a workaround.
+    // FIXME (#2675): assert this works or have a workaround.
     QueryPerformanceFrequency((LARGE_INTEGER *)&_ticks_per_s);
     if (_ticks_per_s == 0LL) {
       _ticks_per_s = 1LL;