Address review: fix merge conflicts with develop

Resolved conflict in unittests/compiled_tests.cpp by keeping both the
Issue #524 unique_ptr vector test and the Issue #607 AST_Node_Trace test.

Requested by @lefticus in PR #648 review.

.github/workflows/ci.yml

@@ -0,0 +1,142 @@
+name: CI
+
+on:
+  push:
+    branches: [develop, main]
+  pull_request:
+    branches: [develop, main]
+  workflow_dispatch:
+
+jobs:
+  linux:
+    name: Linux GCC ${{ matrix.build_type }}
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        build_type: [Debug, Release]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Configure
+        run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }}
+
+      - name: Build
+        run: cmake --build build -j
+
+      - name: Test
+        run: ctest --test-dir build --output-on-failure
+
+  macos:
+    name: macOS AppleClang ${{ matrix.build_type }}
+    runs-on: macos-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        build_type: [Debug, Release]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Configure
+        run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }}
+
+      - name: Build
+        run: cmake --build build -j
+
+      - name: Test
+        run: ctest --test-dir build --output-on-failure
+
+  linux-sanitizers:
+    name: Linux GCC ASAN+UBSAN ${{ matrix.build_type }}
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        build_type: [Debug, Release]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Configure
+        run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_ADDRESS_SANITIZER=ON -DENABLE_UNDEFINED_SANITIZER=ON
+
+      - name: Build
+        run: cmake --build build -j
+
+      - name: Test
+        run: ctest --test-dir build --output-on-failure
+
+  macos-sanitizers:
+    name: macOS AppleClang ASAN+UBSAN ${{ matrix.build_type }}
+    runs-on: macos-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        build_type: [Debug, Release]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Configure
+        run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_ADDRESS_SANITIZER=ON -DENABLE_UNDEFINED_SANITIZER=ON
+
+      - name: Build
+        run: cmake --build build -j
+
+      - name: Test
+        run: ctest --test-dir build --output-on-failure
+
+  windows:
+    name: Windows MSVC ${{ matrix.build_type }}
+    runs-on: windows-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        build_type: [Debug, Release]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Configure
+        run: cmake -B build
+
+      - name: Build
+        run: cmake --build build --config ${{ matrix.build_type }} -j
+
+      - name: Test
+        run: ctest --test-dir build --output-on-failure -C ${{ matrix.build_type }}
+
+  linux-tsan:
+    name: Linux GCC TSAN ${{ matrix.build_type }}
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        build_type: [Debug, Release]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Configure
+        run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON
+
+      - name: Build
+        run: cmake --build build -j
+
+      - name: Test
+        run: ctest --test-dir build --output-on-failure
+
+  macos-tsan:
+    name: macOS AppleClang TSAN ${{ matrix.build_type }}
+    runs-on: macos-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        build_type: [Debug, Release]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Configure
+        run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON
+
+      - name: Build
+        run: cmake --build build -j
+
+      - name: Test
+        run: ctest --test-dir build --output-on-failure

CMakeLists.txt

@@ -34,7 +34,7 @@ if(CMAKE_COMPILER_IS_GNUCC)
   endif()
 endif()
 
-if(CMAKE_COMPILER_IS_GNUCC OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
+if(CMAKE_COMPILER_IS_GNUCC OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
   option(ENABLE_THREAD_SANITIZER "Enable thread sanitizer testing in gcc/clang" FALSE)
   if(ENABLE_THREAD_SANITIZER)
     add_definitions(-fsanitize=thread -g)
@@ -87,6 +87,16 @@ if(CMAKE_COMPILER_IS_GNUCC OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
   endif()
 
 
+elseif(MSVC)
+  option(ENABLE_ADDRESS_SANITIZER "Enable address sanitizer testing in MSVC" FALSE)
+  if(ENABLE_ADDRESS_SANITIZER)
+    add_compile_options(/fsanitize=address)
+    # ASAN is incompatible with /RTC (runtime error checks) and incremental linking
+    string(REGEX REPLACE "/RTC[^ ]*" "" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}")
+    string(REGEX REPLACE "/RTC[^ ]*" "" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}")
+    add_link_options(/INCREMENTAL:NO)
+  endif()
+
 endif()
 
 list(APPEND CPACK_SOURCE_IGNORE_FILES "${CMAKE_CURRENT_BINARY_DIR}")

appveyor.yml

@@ -1,23 +0,0 @@
-version: 6.1.x.{build}
-image:
-  - Visual Studio 2019
-environment:
-  matrix:
-  - VS_VERSION: "Visual Studio 16"
-build_script:
-- cmd: >-
-    mkdir build
-
-    cd build
-
-    cmake c:\Projects\chaiscript -G "%VS_VERSION%" -DBUILD_TESTING:BOOL=ON -DBUILD_MODULES:BOOL=ON
-
-    cmake --build . --config Debug
-test_script:
-- cmd: ctest -C Debug
-notifications:
-- provider: Webhook
-  url: https://webhooks.gitter.im/e/9ff725a985b5679d1d5d
-  on_build_success: true
-  on_build_failure: true
-  on_build_status_changed: false

cheatsheet.md

@@ -153,17 +153,39 @@ This allows you to pass a ChaiScript function to a function requiring `std::vect
 
 ## Adding Objects
 
-```
-chai.add(chaiscript::var(somevar), "somevar"); // copied in
-chai.add(chaiscript::var(std::ref(somevar)), "somevar"); // by reference, shared between C++ and chai
+### `add` — Thread-Local Scoped Variables
+
+`add` adds an object to the current thread's local scope. The variable is only visible in the
+thread that added it. If the variable already exists in the current scope, it is overwritten.
+
+```cpp
+chai.add(chaiscript::var(somevar), "somevar");                    // copied in
+chai.add(chaiscript::var(std::ref(somevar)), "somevar");          // by reference, shared between C++ and chai
 auto shareddouble = std::make_shared<double>(4.3);
-chai.add(chaiscript::var(shareddouble), "shareddouble"); // by shared_ptr, shared between c++ and chai
-chai.add(chaiscript::const_var(somevar), "somevar"); // copied in and made const
-chai.add_global_const(chaiscript::const_var(somevar), "somevar"); // global const. Throws if value is non-const, throws if object exists
-chai.add_global(chaiscript::var(somevar), "somevar"); // global non-const, throws if object exists
-chai.set_global(chaiscript::var(somevar), "somevar"); // global non-const, overwrites existing object
+chai.add(chaiscript::var(shareddouble), "shareddouble");          // by shared_ptr, shared between C++ and chai
+chai.add(chaiscript::const_var(somevar), "somevar");              // copied in and made const
 ```
 
+### `add_global` / `add_global_const` / `set_global` — Global Shared Variables
+
+Global variables are shared between all threads and are visible from any scope (including inside
+functions). Use these when you need a variable accessible everywhere.
+
+```cpp
+chai.add_global_const(chaiscript::const_var(somevar), "somevar"); // global const, throws if value is non-const or object already exists
+chai.add_global(chaiscript::var(somevar), "somevar");             // global non-const, throws if object already exists
+chai.set_global(chaiscript::var(somevar), "somevar");             // global non-const, overwrites existing or creates new
+```
+
+### Summary of Differences
+
+| Method | Scope | Thread Safety | If Name Exists |
+|--------|-------|---------------|----------------|
+| `add` | Thread-local, current scope | Not shared between threads | Overwrites |
+| `add_global` | Global, all scopes and threads | Mutex-protected, shared between threads | Throws exception |
+| `add_global_const` | Global, all scopes and threads | Mutex-protected, shared between threads | Throws exception |
+| `set_global` | Global, all scopes and threads | Mutex-protected, shared between threads | Overwrites |
+
 ## Adding Namespaces
 
 Namespaces will not be populated until `import` is called.

include/chaiscript/language/chaiscript_common.hpp

@@ -26,10 +26,6 @@
 
 namespace chaiscript {
   struct AST_Node;
-  struct AST_Node_Trace;
-  namespace exception {
-    struct eval_error;
-  }
 } // namespace chaiscript
 
 namespace chaiscript {

src/main.cpp

@@ -7,6 +7,7 @@
 // This is an open source non-commercial project. Dear PVS-Studio, please check it.
 // PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
 
+#include <chrono>
 #include <iostream>
 #include <list>
 #include <regex>

unittests/add_vs_add_global.chai

@@ -0,0 +1,43 @@
+// Test demonstrating the difference between add (local scope) and add_global/set_global (global scope)
+// See issue #554
+
+// --- Local variables (var / add) are scoped ---
+// A local variable defined in a block is not visible outside that block
+var local_val = 10
+assert_equal(local_val, 10)
+
+// Local variables can be reassigned in scope
+local_val = 20
+assert_equal(local_val, 20)
+
+// Local variables inside a function are not visible outside
+def set_local() {
+  var func_local = 99
+  assert_equal(func_local, 99)
+}
+set_local()
+
+// --- Globals (add_global) are visible everywhere, including inside functions ---
+var g_val = 42
+add_global(g_val, "my_global")
+
+def check_global() {
+  assert_equal(my_global, 42)
+}
+check_global()
+
+// add_global throws if the global already exists
+assert_throws("Name already exists in current context my_global", fun() { add_global(1, "my_global") })
+
+// --- set_global overwrites an existing global ---
+set_global(100, "my_global")
+assert_equal(my_global, 100)
+
+def check_global_updated() {
+  assert_equal(my_global, 100)
+}
+check_global_updated()
+
+// set_global can also create a new global if it doesn't exist
+set_global(77, "new_global")
+assert_equal(new_global, 77)

unittests/compiled_tests.cpp

@@ -1461,3 +1461,23 @@ TEST_CASE("Issue #524 - vector of unique_ptr can be registered") {
   CHECK(chai.eval<size_t>("var v = UniqueVec(); v.size()") == 0);
   CHECK(chai.eval<bool>("var v2 = UniqueVec(); v2.empty()") == true);
 }
+
+// Regression test for issue #607: AST_Node_Trace must be a complete type
+// when used in eval_error's std::vector<AST_Node_Trace> call_stack member.
+// This failed to compile with C++20 on clang/libc++ when AST_Node_Trace
+// was only forward-declared before eval_error's definition.
+TEST_CASE("eval_error with AST_Node_Trace call stack compiles in C++20") {
+  chaiscript::ChaiScript_Basic chai(create_chaiscript_stdlib(), create_chaiscript_parser());
+
+  // Trigger an eval_error by calling a non-existent function
+  try {
+    chai.eval("nonexistent_function()");
+    REQUIRE(false);
+  } catch (const chaiscript::exception::eval_error &e) {
+    // Verify that eval_error's call_stack member (std::vector<AST_Node_Trace>)
+    // is usable - this would fail to compile if AST_Node_Trace were incomplete
+    const auto &stack = e.call_stack;
+    CHECK(e.pretty_print().size() > 0);
+    (void)stack;
+  }
+}

unittests/now_function.chai

@@ -0,0 +1,3 @@
+// Regression test for issue #660: now() requires <chrono> include
+var t = now()
+assert_true(t > 0)