[compiler] fix warnings in repl lib and add macros to autocomplete (#317)

* fix warnings in repl lib and add macros to autocomplete

* fix crash on ctrl-c, build runtime as static lib and make goos prompt look fancier

* some tweaks for linux build

CMakeLists.txt

@@ -1,8 +1,6 @@
 # Top Level CMakeLists.txt
 cmake_minimum_required(VERSION 3.10)
 
-set(CMAKE_POSITION_INDEPENDENT_CODE ON)
-
 project(jak)
 include(CTest)
 if(NOT CMAKE_BUILD_TYPE)
@@ -64,6 +62,7 @@ include_directories(./)
 include_directories(SYSTEM third-party/inja)
 
 # build repl library
+option(BUILD_SHARED_LIBS "Build shared libraries" ON)
 add_subdirectory(third-party/replxx EXCLUDE_FROM_ALL)
 
 # build common library

common/goos/Interpreter.cpp

@@ -129,9 +129,12 @@ void Interpreter::execute_repl(ReplWrapper& repl) {
   while (!want_exit) {
     try {
       // read something from the user
-      Object obj = reader.read_from_stdin("goos", repl);
+      auto obj = reader.read_from_stdin("goos> ", repl);
+      if (!obj) {
+        continue;
+      }
       // evaluate
-      Object evald = eval_with_rewind(obj, global_environment.as_env());
+      Object evald = eval_with_rewind(*obj, global_environment.as_env());
       // print
       printf("%s\n", evald.print().c_str());
     } catch (std::exception& e) {

common/goos/Reader.cpp

@@ -136,21 +136,28 @@ Reader::Reader() {
 /*!
  * Prompt the user and read the result.
  */
-Object Reader::read_from_stdin(const std::string& prompt, ReplWrapper& repl) {
+std::optional<Object> Reader::read_from_stdin(const std::string& prompt, ReplWrapper& repl) {
   // escape code will make sure that we remove any color
   std::string prompt_full = "\033[0m" + prompt;
-  std::string line = repl.readline(prompt_full);
-  repl.add_to_history(line.c_str());
-  // todo, decide if we should keep reading or not.
 
-  // create text fragment and add to the DB
-  auto textFrag = std::make_shared<ReplText>(line);
-  db.insert(textFrag);
+  auto line_from_repl = repl.readline(prompt_full);
 
-  // perform read
-  auto result = internal_read(textFrag);
-  db.link(result, textFrag, 0);
-  return result;
+  if (line_from_repl) {
+    std::string line = line_from_repl;
+    repl.add_to_history(line.c_str());
+    // todo, decide if we should keep reading or not.
+
+    // create text fragment and add to the DB
+    auto textFrag = std::make_shared<ReplText>(line);
+    db.insert(textFrag);
+
+    // perform read
+    auto result = internal_read(textFrag);
+    db.link(result, textFrag, 0);
+    return result;
+  } else {
+    return {};
+  }
 }
 
 /*!

common/goos/Reader.h

@@ -15,6 +15,7 @@
 #include <cassert>
 #include <utility>
 #include <unordered_map>
+#include <optional>
 
 #include "common/goos/Object.h"
 #include "common/goos/TextDB.h"
@@ -70,7 +71,7 @@ class Reader {
  public:
   Reader();
   Object read_from_string(const std::string& str, bool add_top_level = true);
-  Object read_from_stdin(const std::string& prompt, ReplWrapper& repl);
+  std::optional<Object> read_from_stdin(const std::string& prompt, ReplWrapper& repl);
   Object read_from_file(const std::vector<std::string>& file_path);
 
   std::string get_source_dir();

common/goos/ReplUtils.cpp

@@ -16,7 +16,6 @@ void ReplWrapper::clear_screen() {
 }
 
 void ReplWrapper::print_welcome_message() {
-  clear_screen();
   // Welcome message / brief intro for documentation
   std::string ascii;
   ascii += " _____             _____ _____ _____ __    \n";

common/goos/TextDB.cpp

@@ -150,7 +150,20 @@ void TextDb::inherit_info(const Object& parent, const Object& child) {
   if (parent.is_pair() && child.is_pair()) {
     auto parent_kv = map.find(parent.heap_obj);
     if (parent_kv != map.end()) {
-      map[child.heap_obj] = parent_kv->second;
+      std::vector<const Object*> children = {&child};
+      // mark all forms as children. This will help with error messages in macros, and makes
+      // (add-macro-to-autocomplete) work properly.
+      while (!children.empty()) {
+        auto top = children.back();
+        children.pop_back();
+        map[top->heap_obj] = parent_kv->second;
+        if (top->as_pair()->car.is_pair()) {
+          children.push_back(&top->as_pair()->car);
+        }
+        if (top->as_pair()->cdr.is_pair()) {
+          children.push_back(&top->as_pair()->cdr);
+        }
+      }
     }
   }
 }

doc/changelog.md

@@ -118,4 +118,5 @@
 - There is no longer a separate compiler form for variable vs. constant shifts. Instead the compiler will pick the constant shift automatically when possible. The shifts are called `sar`, `shl` and `shr`, like the x86 instructions.
 - Static type reference link data now has the correct number of methods. This prevents errors like `dkernel: trying to redefine a type 'game-info' with 29 methods when it had 12, try restarting` when you did not actually redefine the number of methods.
 - Added `get-info` to figure out what something is and where it's defined.
-- Added `autocomplete` to get auto-completions for a prefix.
+- Added `autocomplete` to get auto-completions for a prefix.
+- Added `add-macro-to-autocomplete` to add a macro to the auto-completer.

doc/goal_doc.md

@@ -761,6 +761,13 @@ Builds all the DGO files described in the DGO description file. See `goal_src/bu
 
 In the future, this may become part of `asm-data-file`.
 
+## `add-macro-to-autocomplete`
+```lisp
+(add-macro-to-autocomplete macro-name)
+```
+
+Makes the given name show up as a macro in the GOAL REPL. Generating macros may be done programmatically using GOOS and this form can be used to make these show up in the autocomplete. This also makes the macro known to `get-info` which will report that the macro was defined at the location where the macro which expanded to `add-macro-to-autocomplete` is located in GOAL code.  This is used internally by `defmacro`.
+
 # Compiler Forms - Control Flow
 
 ## GOAL "Two Element" Conditions

game/CMakeLists.txt

@@ -76,13 +76,13 @@ set(RUNTIME_SOURCE
 
 
 # we build the runtime as a static library.
-add_library(runtime ${RUNTIME_SOURCE} "../third-party/glad/src/glad.c")
+add_library(runtime STATIC ${RUNTIME_SOURCE} "../third-party/glad/src/glad.c")
 
 target_link_libraries(runtime common fmt glfw)
 if(WIN32)
     target_link_libraries(runtime mman)
 else()
-    target_link_libraries(runtime pthread)
+    target_link_libraries(runtime pthread dl)
 endif()
 
 add_executable(gk main.cpp)

game/graphics/display.cpp

@@ -11,7 +11,7 @@ namespace Display {
 
 GLFWwindow* display = NULL;
 
-void InitDisplay(int width, int height, char* title, GLFWwindow*& d) {
+void InitDisplay(int width, int height, const char* title, GLFWwindow*& d) {
   if (d) {
     lg::warn("InitDisplay has already created a display window");
     return;

game/graphics/display.h

@@ -17,7 +17,7 @@ namespace Display {
 // a single window.
 extern GLFWwindow* display;
 
-void InitDisplay(int width, int height, char* title, GLFWwindow*& d);
+void InitDisplay(int width, int height, const char* title, GLFWwindow*& d);
 void KillDisplay(GLFWwindow*& d);
 
 }  // namespace Display

goal_src/goos-lib.gs

@@ -145,15 +145,18 @@
 
 ;; goal macro to define a goal macro
 (defgmacro defmacro (name args &rest body)
-    (if (and
-        (> (length body) 1)      ;; more than one thing in function
-        (string? (first body))   ;; first thing is a string
-        )
-    ;; then it's a docstring and we ignore it.
-    `(seval (defgmacro ,name ,args ,@(cdr body)))
-    ;; otherwise don't ignore it.
-    `(seval (defgmacro ,name ,args ,@body))
-    )
+  `(begin
+     (add-macro-to-autocomplete ,name)
+     ,(if (and
+           (> (length body) 1) ;; more than one thing in function
+           (string? (first body)) ;; first thing is a string
+           )
+          ;; then it's a docstring and we ignore it.
+          `(seval (defgmacro ,name ,args ,@(cdr body)))
+          ;; otherwise don't ignore it.
+          `(seval (defgmacro ,name ,args ,@body))
+          )
+     )
   )
 
 ;; goal macro to define a goos macro

goalc/compiler/Compiler.cpp

@@ -53,10 +53,13 @@ ReplStatus Compiler::execute_repl() {
         prompt = fmt::format(fmt::emphasis::bold | fg(fmt::color::red), "gr> ");
       }
 
-      Object code = m_goos.reader.read_from_stdin(prompt, *m_repl.get());
+      auto code = m_goos.reader.read_from_stdin(prompt, *m_repl.get());
+      if (!code) {
+        continue;
+      }
 
       // 2). compile
-      auto obj_file = compile_object_file("repl", code, m_listener.is_connected());
+      auto obj_file = compile_object_file("repl", *code, m_listener.is_connected());
       if (m_settings.debug_print_ir) {
         obj_file->debug_print_tl();
       }

goalc/compiler/Compiler.h

@@ -449,6 +449,9 @@ class Compiler {
   Val* compile_reload(const goos::Object& form, const goos::Object& rest, Env* env);
   Val* compile_get_info(const goos::Object& form, const goos::Object& rest, Env* env);
   Val* compile_autocomplete(const goos::Object& form, const goos::Object& rest, Env* env);
+  Val* compile_add_macro_to_autocomplete(const goos::Object& form,
+                                         const goos::Object& rest,
+                                         Env* env);
 
   // ControlFlow
   Condition compile_condition(const goos::Object& condition, Env* env, bool invert);

goalc/compiler/compilation/Atoms.cpp

@@ -119,6 +119,7 @@ const std::unordered_map<
         {"reload", &Compiler::compile_reload},
         {"get-info", &Compiler::compile_get_info},
         {"autocomplete", &Compiler::compile_autocomplete},
+        {"add-macro-to-autocomplete", &Compiler::compile_add_macro_to_autocomplete},
 
         // CONDITIONAL COMPILATION
         {"#cond", &Compiler::compile_gscond},

goalc/compiler/compilation/CompilerControl.cpp

@@ -188,7 +188,7 @@ Val* Compiler::compile_asm_file(const goos::Object& form, const goos::Object& re
     printf("\n");
   } else {
     auto total_time = total_timer.getMs();
-    if (total_time > 10.0) {
+    if (total_time > 10.0 && color) {
       fmt::print("[ASM-FILE] {} took {:.2f} ms\n", obj_file_name, total_time);
     }
   }
@@ -239,9 +239,7 @@ Val* Compiler::compile_listen_to_target(const goos::Object& form,
   return get_none();
 }
 
-Val* Compiler::compile_repl_clear_screen(const goos::Object& form,
-                                         const goos::Object& rest,
-                                         Env* env) {
+Val* Compiler::compile_repl_clear_screen(const goos::Object&, const goos::Object&, Env*) {
   m_repl.get()->clear_screen();
   return get_none();
 }
@@ -410,6 +408,8 @@ Val* Compiler::compile_get_info(const goos::Object& form, const goos::Object& re
 Replxx::completions_t Compiler::find_symbols_by_prefix(std::string const& context,
                                                        int& contextLen,
                                                        std::vector<std::string> const& user_data) {
+  (void)contextLen;
+  (void)user_data;
   auto token = m_repl.get()->get_current_repl_token(context);
   auto possible_forms = m_symbol_info.lookup_symbols_starting_with(token.first);
   Replxx::completions_t completions;
@@ -423,6 +423,8 @@ Replxx::hints_t Compiler::find_hints_by_prefix(std::string const& context,
                                                int& contextLen,
                                                Replxx::Color& color,
                                                std::vector<std::string> const& user_data) {
+  (void)contextLen;
+  (void)user_data;
   auto token = m_repl.get()->get_current_repl_token(context);
   auto possible_forms = m_symbol_info.lookup_symbols_starting_with(token.first);
 
@@ -447,6 +449,7 @@ void Compiler::repl_coloring(
     std::string const& context,
     Replxx::colors_t& colors,
     std::vector<std::pair<std::string, Replxx::Color>> const& regex_color) {
+  (void)regex_color;
   using cl = Replxx::Color;
   // TODO - a proper circular queue would be cleaner to use
   std::deque<cl> paren_colors = {cl::GREEN, cl::CYAN, cl::MAGENTA};
@@ -465,7 +468,7 @@ void Compiler::repl_coloring(
       std::vector<SymbolInfo>* sym_match = m_symbol_info.lookup_exact_name(curr_symbol.second);
       if (sym_match != nullptr && sym_match->size() == 1) {
         SymbolInfo sym_info = sym_match->at(0);
-        for (int pos = curr_symbol.first; pos <= i; pos++) {
+        for (int pos = curr_symbol.first; pos <= int(i); pos++) {
           // TODO - currently just coloring all types brown/gold
           // - would be nice to have a different color for globals, functions, etc
           colors.at(pos) = cl::BROWN;
@@ -527,3 +530,13 @@ Val* Compiler::compile_autocomplete(const goos::Object& form, const goos::Object
 
   return get_none();
 }
+
+Val* Compiler::compile_add_macro_to_autocomplete(const goos::Object& form,
+                                                 const goos::Object& rest,
+                                                 Env* env) {
+  (void)env;
+  auto args = get_va(form, rest);
+  va_check(form, args, {goos::ObjectType::SYMBOL}, {});
+  m_symbol_info.add_macro(args.unnamed.at(0).as_symbol()->name, form);
+  return get_none();
+}

third-party/replxx/src/conversion.cxx

@@ -20,6 +20,11 @@ void to_lower( std::string& s_ ) {
 	transform( s_.begin(), s_.end(), s_.begin(), static_cast<int(*)(int)>( &tolower ) );
 }
 
+#ifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif
+
 bool is_8bit_encoding( void ) {
 	bool is8BitEncoding( false );
 	string origLC( setlocale( LC_CTYPE, nullptr ) );
@@ -37,6 +42,10 @@ bool is_8bit_encoding( void ) {
 	return ( is8BitEncoding );
 }
 
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
 bool is8BitEncoding( is_8bit_encoding() );
 
 }

third-party/replxx/src/replxx_impl.cxx

@@ -942,6 +942,11 @@ int longest_common_prefix( Replxx::ReplxxImpl::completions_t const& completions
 }
 }
 
+#ifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif
+
 /**
  * Handle command completion, using a completionCallback() routine to provide
  * possible substitutions
@@ -1166,6 +1171,10 @@ char32_t Replxx::ReplxxImpl::do_complete_line( bool showCompletions_ ) {
 	return 0;
 }
 
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
 int Replxx::ReplxxImpl::get_input_line( void ) {
 	// The latest history entry is always our current buffer
 	if ( _data.length() > 0 ) {