cleanups from code review

diff --git a/basis/cpu/x86/64/bootstrap.factor b/basis/cpu/x86/64/bootstrap.factor
index e32fcf7200564e11d5d90f28565cd51f4c4489f0..0b013e82605ccfe32ffa67ea64d9f523e82df226 100755 (executable)
--- a/basis/cpu/x86/64/bootstrap.factor
+++ b/basis/cpu/x86/64/bootstrap.factor
@@ -96,7 +96,6 @@ IN: bootstrap.x86
 : signal-handler-save-regs ( -- regs ) volatile-regs R12 suffix ;
 
 :: jit-signal-handler-prolog ( -- frame-size )
-    ! do we also need to save XMM?
     signal-handler-save-regs :> save-regs
     save-regs length bootstrap-cells 16 align stack-frame-size + :> frame-size
     RSP frame-size bootstrap-cell - SUB ! minus a cell for return address

diff --git a/vm/code_heap.cpp b/vm/code_heap.cpp
index f8b78eabde17f7ddd9014e4d3ee62f90303fec07..c81cef10e194750fa1ecaf114aaadb64259a146d 100755 (executable)
--- a/vm/code_heap.cpp
+++ b/vm/code_heap.cpp
@@ -102,11 +102,6 @@ void factor_vm::init_code_heap(cell size)
        code = new code_heap(size);
 }
 
-bool factor_vm::in_code_heap_p(cell ptr)
-{
-       return (ptr >= code->seg->start && ptr <= code->seg->end);
-}
-
 struct word_updater {
        factor_vm *parent;
        bool reset_inline_caches;

diff --git a/vm/cpu-x86.cpp b/vm/cpu-x86.cpp
index 4d09a132230f32bba15daff3605aa9706cde1bf2..c27e563a7c654325eb5b524ede5581fe74abd22d 100644 (file)
--- a/vm/cpu-x86.cpp
+++ b/vm/cpu-x86.cpp
@@ -7,10 +7,10 @@ void factor_vm::dispatch_signal_handler(cell *sp, cell *pc, cell handler)
 {
        if (!code->seg->in_segment_p(*pc) || *sp < ctx->callstack_seg->start + stack_reserved)
        {
-               /* Fault came from foreign code, a callstack overflow, or we would probably
-               overflow if we tried the resumable handler. We can't resume, so cut the
+               /* Fault came from foreign code or a callstack overflow, or we don't
+               have enough callstack room to try the resumable handler. Cut the
                callstack down to the shallowest Factor stack frame that leaves room for
-               the signal handler to do its thing and launch the handler without going
+               the signal handler to do its thing, and launch the handler without going
                through the resumable subprimitive. */
                signal_resumable = false;
                stack_frame *frame = ctx->callstack_bottom - 1;
@@ -61,9 +61,7 @@ void factor_vm::dispatch_signal_handler(cell *sp, cell *pc, cell handler)
                        handler_word = tagged<word>(special_objects[LEAF_SIGNAL_HANDLER_WORD]);
                }
                else
-               {
-                       fatal_error("Invalid stack frame during signal handler", *sp);
-               }
+                       assert(false);
 
                *pc = (cell)handler_word->code->entry_point();
        }

diff --git a/vm/debug.cpp b/vm/debug.cpp
index 4cee4c74b7b8d05e319c398dab3acdbe23d7d22d..314998cd1285a33d5eae6a3486ba32f7716044f2 100755 (executable)
--- a/vm/debug.cpp
+++ b/vm/debug.cpp
@@ -144,13 +144,13 @@ void factor_vm::print_objects(cell *start, cell *end)
 
 void factor_vm::print_datastack()
 {
-       std::cout << "==== DATA STACK:\n";
+       std::cout << "==== DATA STACK:" << std::endl;
        print_objects((cell *)ctx->datastack_seg->start,(cell *)ctx->datastack);
 }
 
 void factor_vm::print_retainstack()
 {
-       std::cout << "==== RETAIN STACK:\n";
+       std::cout << "==== RETAIN STACK:" << std::endl;
        print_objects((cell *)ctx->retainstack_seg->start,(cell *)ctx->retainstack);
 }
 
@@ -181,7 +181,7 @@ struct stack_frame_printer {
 
 void factor_vm::print_callstack()
 {
-       std::cout << "==== CALL STACK:\n";
+       std::cout << "==== CALL STACK:" << std::endl;
        stack_frame_printer printer(this);
        iterate_callstack(ctx,printer);
 }
@@ -339,41 +339,41 @@ void factor_vm::dump_code_heap()
 {
        code_block_printer printer(this);
        code->allocator->iterate(printer);
-       std::cout << printer.reloc_size << " bytes used by relocation tables\n";
-       std::cout << printer.parameter_size << " bytes used by parameter tables\n";
+       std::cout << printer.reloc_size << " bytes used by relocation tables" << std::endl;
+       std::cout << printer.parameter_size << " bytes used by parameter tables" << std::endl;
 }
 
 void factor_vm::factorbug()
 {
        if(fep_disabled)
        {
-               std::cout << "Low level debugger disabled\n";
+               std::cout << "Low level debugger disabled" << std::endl;
                exit(1);
        }
 
        fep_p = true;
 
-       std::cout << "Starting low level debugger...\n";
-       std::cout << "Basic commands:\n";
-       std::cout << "  q ^D             -- quit Factor\n";
-       std::cout << "  c                -- continue executing Factor - NOT SAFE\n";
-       std::cout << "  t                -- throw exception in Factor - NOT SAFE\n";
-       std::cout << "  .s .r .c         -- print data, retain, call stacks\n";
-       std::cout << "Advanced commands:\n";
-       std::cout << "  e                -- dump environment\n";
-       std::cout << "  d <addr> <count> -- dump memory\n";
-       std::cout << "  u <addr>         -- dump object at tagged <addr>\n";
-       std::cout << "  . <addr>         -- print object at tagged <addr>\n";
-       std::cout << "  g                -- dump generations\n";
-       std::cout << "  ds dr            -- dump data, retain stacks\n";
-       std::cout << "  trim             -- toggle output trimming\n";
-       std::cout << "  data             -- data heap dump\n";
-       std::cout << "  words            -- words dump\n";
-       std::cout << "  tuples           -- tuples dump\n";
-       std::cout << "  refs <addr>      -- find data heap references to object\n";
-       std::cout << "  push <addr>      -- push object on data stack - NOT SAFE\n";
-       std::cout << "  gc               -- trigger full GC - NOT SAFE\n";
-       std::cout << "  code             -- code heap dump\n";
+       std::cout << "Starting low level debugger..." << std::endl;
+       std::cout << "Basic commands:" << std::endl;
+       std::cout << "  q ^D             -- quit Factor" << std::endl;
+       std::cout << "  c                -- continue executing Factor - NOT SAFE" << std::endl;
+       std::cout << "  t                -- throw exception in Factor - NOT SAFE" << std::endl;
+       std::cout << "  .s .r .c         -- print data, retain, call stacks" << std::endl;
+       std::cout << "Advanced commands:" << std::endl;
+       std::cout << "  e                -- dump environment" << std::endl;
+       std::cout << "  d <addr> <count> -- dump memory" << std::endl;
+       std::cout << "  u <addr>         -- dump object at tagged <addr>" << std::endl;
+       std::cout << "  . <addr>         -- print object at tagged <addr>" << std::endl;
+       std::cout << "  g                -- dump generations" << std::endl;
+       std::cout << "  ds dr            -- dump data, retain stacks" << std::endl;
+       std::cout << "  trim             -- toggle output trimming" << std::endl;
+       std::cout << "  data             -- data heap dump" << std::endl;
+       std::cout << "  words            -- words dump" << std::endl;
+       std::cout << "  tuples           -- tuples dump" << std::endl;
+       std::cout << "  refs <addr>      -- find data heap references to object" << std::endl;
+       std::cout << "  push <addr>      -- push object on data stack - NOT SAFE" << std::endl;
+       std::cout << "  gc               -- trigger full GC - NOT SAFE" << std::endl;
+       std::cout << "  code             -- code heap dump" << std::endl;
 
        bool seen_command = false;
 
@@ -381,7 +381,7 @@ void factor_vm::factorbug()
        {
                char cmd[1024];
 
-               std::cout << "READY\n";
+               std::cout << "READY" << std::endl;
                std::cout.flush();
 
                std::cin >> std::setw(1024) >> cmd >> std::setw(0); 
@@ -464,7 +464,7 @@ void factor_vm::factorbug()
                else if(strcmp(cmd,"refs") == 0)
                {
                        cell addr = read_cell_hex();
-                       std::cout << "Data heap references:\n";
+                       std::cout << "Data heap references:" << std::endl;
                        find_data_references(addr);
                        std::cout << std::endl;
                }
@@ -482,14 +482,14 @@ void factor_vm::factorbug()
                else if(strcmp(cmd,"gc") == 0)
                        primitive_full_gc();
                else
-                       std::cout << "unknown command\n";
+                       std::cout << "unknown command" << std::endl;
        }
 }
 
 void factor_vm::primitive_die()
 {
-       std::cout << "The die word was called by the library. Unless you called it yourself,\n";
-       std::cout << "you have triggered a bug in Factor. Please report.\n";
+       std::cout << "The die word was called by the library. Unless you called it yourself," << std::endl;
+       std::cout << "you have triggered a bug in Factor. Please report." << std::endl;
        factorbug();
 }
 

diff --git a/vm/errors.cpp b/vm/errors.cpp
index bfe0ee91b14376ea68d8962ceed59d6256376e11..340ccbaa1a974d5472559269ee7322bc56bd3c7c 100755 (executable)
--- a/vm/errors.cpp
+++ b/vm/errors.cpp
@@ -172,15 +172,15 @@ void factor_vm::enqueue_safepoint_fep()
 
 void factor_vm::enqueue_safepoint_sample()
 {
-       if (!sampling_p)
-               fatal_error("Received sampling signal while not sampling!", 0);
-       ++safepoint_sample_count;
+       if (sampling_profiler_p)
+               ++safepoint_sample_count;
 }
 
 void factor_vm::handle_safepoint()
 {
        code->unguard_safepoint();
-       if (safepoint_fep) {
+       if (safepoint_fep)
+       {
                std::cout << "Interrupted\n";
                factorbug();
                safepoint_fep = false;

diff --git a/vm/os-unix.cpp b/vm/os-unix.cpp
index e007b574ff0bcc60b50ca060017355738d57b50a..90324de03e68cf8001186c6e576e827e33d64d31 100755 (executable)
--- a/vm/os-unix.cpp
+++ b/vm/os-unix.cpp
@@ -168,7 +168,7 @@ void synchronous_signal_handler(int signal, siginfo_t *siginfo, void *uap)
                vm->signal_number = signal;
                vm->dispatch_signal(uap,factor::synchronous_signal_handler_impl);
        } else
-               fatal_error("Foreign thread received signal ", signal);
+               fatal_error("Foreign thread received signal", signal);
 }
 
 void enqueue_signal_handler(int signal, siginfo_t *siginfo, void *uap)
@@ -177,7 +177,7 @@ void enqueue_signal_handler(int signal, siginfo_t *siginfo, void *uap)
        if (vm)
                vm->enqueue_safepoint_signal(signal);
        else
-               fatal_error("Foreign thread received signal ", signal);
+               fatal_error("Foreign thread received signal", signal);
 }
 
 void fep_signal_handler(int signal, siginfo_t *siginfo, void *uap)
@@ -186,7 +186,7 @@ void fep_signal_handler(int signal, siginfo_t *siginfo, void *uap)
        if (vm)
                vm->enqueue_safepoint_fep();
        else
-               fatal_error("Foreign thread received signal ", signal);
+               fatal_error("Foreign thread received signal", signal);
 }
 
 void sample_signal_handler(int signal, siginfo_t *siginfo, void *uap)
@@ -195,7 +195,7 @@ void sample_signal_handler(int signal, siginfo_t *siginfo, void *uap)
        if (vm)
                vm->enqueue_safepoint_sample();
        else
-               fatal_error("Foreign thread received signal ", signal);
+               fatal_error("Foreign thread received signal", signal);
 }
 
 void ignore_signal_handler(int signal, siginfo_t *siginfo, void *uap)

diff --git a/vm/os-windows.cpp b/vm/os-windows.cpp
index 4f695ae0211d71e9a7f0e4bbd25dab3aae90100f..3cbfc7e85c55e1ab37aba13f4aea429d3fd9ef78 100755 (executable)
--- a/vm/os-windows.cpp
+++ b/vm/os-windows.cpp
@@ -271,7 +271,7 @@ VM_C_API LONG exception_handler(PEXCEPTION_RECORD e, void *frame, PCONTEXT c, vo
                return vm->exception_handler(e,frame,c,dispatch);
        else
        {
-               fatal_error("Foreign thread received exception ", e->ExceptionCode);
+               fatal_error("Foreign thread received exception", e->ExceptionCode);
                return 0; // to placate MSVC
        }
 }
@@ -281,7 +281,7 @@ static BOOL WINAPI ctrl_handler(DWORD dwCtrlType)
        switch (dwCtrlType) {
        case CTRL_C_EVENT:
        case CTRL_BREAK_EVENT:
-               {
+       {
                /* The CtrlHandler runs in its own thread without stopping the main thread.
                Since in practice nobody uses the multi-VM stuff yet, we just grab the first
                VM we can get. This will not be a good idea when we actually support native
@@ -290,7 +290,7 @@ static BOOL WINAPI ctrl_handler(DWORD dwCtrlType)
                factor_vm *vm = thread_vms.begin()->second;
                vm->enqueue_safepoint_fep();
                return TRUE;
-               }
+       }
        default:
                return FALSE;
        }

diff --git a/vm/profiler.cpp b/vm/profiler.cpp
index ef717e0fc6ea059991d39685681fe3c1a5fc88bf..8512230d929922e95f80ff72ae767670ef49b9a6 100755 (executable)
--- a/vm/profiler.cpp
+++ b/vm/profiler.cpp
@@ -5,7 +5,7 @@ namespace factor
 
 void factor_vm::init_profiler()
 {
-       profiling_p = false;
+       counting_profiler_p = false;
 }
 
 /* Allocates memory */
@@ -22,7 +22,7 @@ code_block *factor_vm::compile_profiling_stub(cell word_)
 /* Allocates memory */
 void factor_vm::set_profiling(bool profiling)
 {
-       if(profiling == profiling_p)
+       if(profiling == counting_profiler_p)
                return;
 
        /* Push everything to tenured space so that we can heap scan
@@ -31,7 +31,7 @@ void factor_vm::set_profiling(bool profiling)
 
        data_root<array> words(find_all_words(),this);
 
-       profiling_p = profiling;
+       counting_profiler_p = profiling;
 
        cell length = array_capacity(words.untagged());
        for(cell i = 0; i < length; i++)

diff --git a/vm/vm.cpp b/vm/vm.cpp
index 4701a5024abf0c585877db120ea5d13f9e145ac9..a1da97cd6d4b156cf9e1958321999703abf20a80 100755 (executable)
--- a/vm/vm.cpp
+++ b/vm/vm.cpp
@@ -7,8 +7,8 @@ factor_vm::factor_vm() :
        nursery(0,0),
        callback_id(0),
        c_to_factor_func(NULL),
-       profiling_p(false),
-       sampling_p(false),
+       counting_profiler_p(false),
+       sampling_profiler_p(false),
        safepoint_fep(false),
        safepoint_sample_count(0),
        gc_off(false),

diff --git a/vm/vm.hpp b/vm/vm.hpp
index 22720632d817590d72068d4dd83d1078f991ce54..a0914492b501cebf179dfe0926cf0706533e7253 100755 (executable)
--- a/vm/vm.hpp
+++ b/vm/vm.hpp
@@ -61,9 +61,9 @@ struct factor_vm
        c_to_factor_func_type c_to_factor_func;
 
        /* Is call counting enabled? */
-       bool profiling_p;
+       bool counting_profiler_p;
        /* Is sampling profiler enabled? */
-       bool sampling_p;
+       bool sampling_profiler_p;
 
        /* Global variables used to pass fault handler state from signal handler
        to VM */
@@ -350,16 +350,11 @@ struct factor_vm
                return (Type *)allot_object(Type::type_number,size);
        }
 
-       inline bool in_data_heap_p(cell pointer)
-       {
-               return (pointer >= data->seg->start && pointer < data->seg->end);
-       }
-
        inline void check_data_pointer(object *pointer)
        {
        #ifdef FACTOR_DEBUG
                if(!(current_gc && current_gc->op == collect_growing_heap_op))
-                       assert(in_data_heap_p((cell)pointer));
+                       assert(data->seg->in_segment_p((cell)pointer));
        #endif
        }
 
@@ -566,7 +561,6 @@ struct factor_vm
        }
 
        void init_code_heap(cell size);
-       bool in_code_heap_p(cell ptr);
        void update_code_heap_words(bool reset_inline_caches);
        void initialize_code_blocks();
        void primitive_modify_code_heap();

diff --git a/vm/words.cpp b/vm/words.cpp
index 243e476f948a57c1ab6446fc61d39366c918e2a7..2fd6fa42dcc05a507c89970a816052d00e5e8b84 100644 (file)
--- a/vm/words.cpp
+++ b/vm/words.cpp
@@ -62,7 +62,7 @@ word *factor_vm::allot_word(cell name_, cell vocab_, cell hashcode_)
        new_word->code = NULL;
 
        jit_compile_word(new_word.value(),new_word->def,true);
-       if(profiling_p)
+       if(counting_profiler_p)
        {
                code_block *profiling_block = compile_profiling_stub(new_word.value());
                new_word->profiling = profiling_block;
@@ -89,7 +89,7 @@ void factor_vm::primitive_word_code()
        data_root<word> w(ctx->pop(),this);
        w.untag_check(this);
 
-       if(profiling_p)
+       if(counting_profiler_p)
        {
                ctx->push(from_unsigned_cell((cell)w->profiling->entry_point()));
                ctx->push(from_unsigned_cell((cell)w->profiling + w->profiling->size()));
@@ -103,7 +103,7 @@ void factor_vm::primitive_word_code()
 
 void factor_vm::update_word_entry_point(word *w)
 {
-       if(profiling_p && w->profiling)
+       if(counting_profiler_p && w->profiling)
                w->entry_point = w->profiling->entry_point();
        else
                w->entry_point = w->code->entry_point();