Adding basic namespace handling

[stephenberry]

Issue this pull request references: #113

• I posted a more detailed explanation about this pull request in the discussion: Add namespacing #113.

The new code is wrapped in NAMESPACE HANDLING comments.

C++:
register_namespace(): registers a namespace with an instance of ChaiScript (supports delayed namespace generation)
import(): imports a namespace as a global Dynamic_Object

ChaiScript:
import(): imports a namespace
namespace(): generates and registers a new namespace

[lefticus]

• Please add { } in the if statements where you've left them off, and add mutex protects (notes have been made on both of these)
• Please add a unit test showing the expected behavior of these functions in the unit_tests folder https://github.com/ChaiScript/ChaiScript/tree/develop/unittests

[lefticus]

All if() statements should have { } for safety and code clarity

[lefticus]

This code particularly makes my point about { } being always required - mixed if/else if/if without { } can add to confusion during later edits

[lefticus]

All of these functions will need mutex protection, similar to this (note that it only needs m_mutex, not m_use_mutex): https://github.com/stephenberry/ChaiScript/blob/a01687d7ad836c24621d8af26c08d760bae7eb0d/include/chaiscript/language/chaiscript_engine.hpp#L467

[stephenberry]

I've started to implement your requested changes, but I ran into an issue when adding the mutex to all the register_namespace functions. I haven't looked into why yet.
I'll also add a unit test when I get a chance.

[lefticus]

Is it a compile time or run time error you are getting?

[stephenberry]

It's a run time error: device or resource busy: device or resource busy

[lefticus]

You're hitting a recursive mutex lock probably, don't worry about that part now, instead focus on the tests/examples.

[stephenberry]

OK, I'll work on tests/examples.

[stephenberry]

I added three unit tests for namespaces as well as documentation to the cheatsheet. In the cheatsheet I addressed namespaces both in the section for adding things to the engine and immediately after the section on Dynamic Objects.

[stephenberry]

Just let me know if there is anything else you'd like me to consider or add for namespaces. My aim was to add as little code as possible while providing a framework for namespaces.

[lefticus]

Sorry, I'm a little behind on ChaiScript stuff right now, I'll probably get back to this in another week.

[lefticus]

There's a lot I like about this. I will have to address the threading concerns, but I can do that.

I was wondering what you thought about changing the signature of the delayed namespace registration from:

[]() {
   chaiscript::Namespace math;
    math["pi"] = chaiscript::const_var(3.14159);
    math["sin"] = chaiscript::var(chaiscript::fun([](const double x) { return sin(x); }));
    return math; 
}

To something like:

[](chaiscript::Namespace &math) {
    math["pi"] = chaiscript::const_var(3.14159);
    math["sin"] = chaiscript::var(chaiscript::fun([](const double x) { return sin(x); }));
}

So there's less ambiguity about what needs to be done?

-Jason

[stephenberry]

I'm for this change, and not just because it is more straightforward. I think it allows for more flexibility in the future. By passing in a reference we could use this lambda function to expand a namespace rather than merely initialize one, I'm not sure if this is a direction to take namespace handling, but it leaves the possibility open.

Note: With this change the namespace will need to be instantiated in the import method.

[lefticus]

@stephenberry I have not forgotten about this branch, but I haven't had the time I want to devote to it yet. This is the kind of feature that once I commit it people will rely on it, so I want to make sure we are moving forward with something that makes sense for the long term.

[stephenberry]

I understand. I'm looking forward to it, but there's no rush.

[germandiagogomez]

I am one of those users that would love to see this in. 👍 Any progress?

[lefticus]

I feel like now that I've gotten past the hurdle of getting 6.0 out, I can start looking at stuff like this again.

[lefticus]

@stephenberry it has been 189 days since this pull request was last updated.

[stephenberry]

@lefticus I'll update this pull request soon and look once more into possible improvements to the namespacing code.

[codecov-io]

Codecov Report

Merging #290 into develop will increase coverage by <.01%.
The diff coverage is 64.28%.

@@             Coverage Diff             @@
##           develop     #290      +/-   ##
===========================================
+ Coverage    67.79%   67.79%   +<.01%     
===========================================
  Files           59       59              
  Lines        10229    10245      +16     
  Branches      1284     1286       +2     
===========================================
+ Hits          6935     6946      +11     
- Misses        2416     2419       +3     
- Partials       878      880       +2

Impacted Files	Coverage Δ
include/chaiscript/language/chaiscript_engine.hpp	84.07% <64.28%> (-1.31%)	⬇️
include/chaiscript/dispatchkit/boxed_number.hpp	61.92% <0%> (+0.19%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2662395...037fadd. Read the comment docs.

[stephenberry]

@lefticus I've updated the namespacing code and I believe it is in a good state now.

I've simplified the code and the user interface. I'm requiring the user to pass in a function in order to always delay the namespace instantiation. This way the namespace will only be populated if it is imported, which can have significant memory and computation savings.

I also implemented your suggested change on passing a Namespace reference into the register_namespace method.

Here's a simple example:

chai->register_namespace([](chaiscript::Namespace& math) {
      math["pi"] = const_var(3.14159);
      math["sin"] = var(fun([](const double x) { return sin(x); })); },
      "math");

Some ChaiScript example code:

import("math")

print(math.pi) // math namespace defined in C++
print(math.sin(math.pi / 2.0))

namespace("trig") // define a new trig namespace

trig.pi = math.pi

print(2.0*trig.pi)

trig.sin = fun(x) { math.sin(x) } // create a new function that uses the math.sin function
print(trig.sin(math.pi / 2.0))

Let me know if you would like me to consider anything else concerning namespacing. If we can get this into the main ChaiScript repo then I'll be able to implement it into more of my work and test it in broader applications.

[lefticus]

@stephenberry I went for it. Sorry for the supremely long delay and thank you for the hard work.

[stephenberry]

@lefticus Excellent, thanks! I'm glad I could contribute. I'll certainly be using this.

[RobLoach]

Nice work on this, you two! Will check it out soon.