From b94b49bf8624684ac43a202a5ba6e305141a7fe9 Mon Sep 17 00:00:00 2001 From: scottolsen Date: Fri, 19 Jun 2020 10:30:55 -0400 Subject: [PATCH 1/4] Implement private Even though `private?` has been around for a while, and we document the behavior of `private` as marking a binding as private to a module--it was never implemented as far as I can tell. This implements private by adding a simple check to the evaluator. If a binding is found in the global context, we check if it's marked as private. If so, we inform the user that it can't be called from outside of its module. Note that we don't perform this check if the binding is found in the internal env, since that means it's a function called within the same module and thus is ok (even if marked as private). After this change, something like the following works, granting us proper encapsulation: ``` ;; File Foo.carp (deftype Foo [bar Int]) (private Foo.bar) (defmodule Foo (defn get [foo] (Foo.bar foo)) ) ;; Enter REPL (load "Foo.carp") (Foo.bar &(Foo.init 1)) The binding: Foo.bar is private; it may only be used within the module that defines it. at REPL:1:2. @(Foo.get &(Foo.init 1)) Compiled to 'out/Untitled' (executable) 1 => 0 ``` N.B. I also had to remove a private declaration from fmt-internal--this declaration didn't really make much sense anyway, as fmt-internal is a global function, so module-based privacy is not enforceable. --- core/Format.carp | 1 - src/Eval.hs | 5 ++++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/Format.carp b/core/Format.carp index 5d8c412f..571b104e 100644 --- a/core/Format.carp +++ b/core/Format.carp @@ -1,4 +1,3 @@ -(private fmt-internal) (hidden fmt-internal) (defndynamic fmt-internal [s args] (let [idx (String.index-of s \%) diff --git a/src/Eval.hs b/src/Eval.hs index 6658514a..875bce70 100644 --- a/src/Eval.hs +++ b/src/Eval.hs @@ -62,7 +62,10 @@ eval ctx xobj@(XObj o i t) = Nothing -> Nothing tryLookup path = case lookupInEnv path (contextGlobalEnv ctx) of - Just (_, Binder _ found) -> return (ctx, Right (resolveDef found)) + Just (_, Binder meta found) -> + if metaIsTrue meta "private" + then return (evalError ctx ("The binding: " ++ show (getPath found) ++ " is private; it may only be used within the module that defines it.") i) + else return (ctx, Right (resolveDef found)) Nothing -> case lookupInEnv path (getTypeEnv (contextTypeEnv ctx)) of Just (_, Binder _ found) -> return (ctx, Right (resolveDef found)) From c5857c74c610dcf05190af475930f9db39fbec11 Mon Sep 17 00:00:00 2001 From: scottolsen Date: Fri, 19 Jun 2020 14:37:10 -0400 Subject: [PATCH 2/4] Add privacy checking to expansions The previous commit added privacy checking to the evaluator--as it turns out, this is only sufficient for top-level forms--interior forms, e.g. forms in a `defn` are examined by `Expand` via `expandAll` and never reach the `Eval` check. To remedy this, I've added the check to symbol expansions. I've also tweaked the signature of `expandSymbol` to align it with other functions (e.g. `expandArray`). --- src/Expand.hs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/Expand.hs b/src/Expand.hs index bee0cb9f..e30a4cb5 100644 --- a/src/Expand.hs +++ b/src/Expand.hs @@ -36,7 +36,7 @@ expand eval ctx xobj = --case obj (trace ("Expand: " ++ pretty xobj) xobj) of Lst _ -> expandList xobj Arr _ -> expandArray xobj - Sym _ _ -> return (ctx, expandSymbol xobj) + Sym _ _ -> expandSymbol xobj _ -> return (ctx, Right xobj) where @@ -169,18 +169,22 @@ expand eval ctx xobj = Right (XObj (Arr okXObjs) i t)) expandArray _ = error "Can't expand non-array in expandArray." - expandSymbol :: XObj -> Either a XObj - expandSymbol (XObj (Sym path _) _ _) = + expandSymbol :: XObj -> IO (Context, Either EvalError XObj) + expandSymbol sym@(XObj (Sym path _) _ _) = case lookupInEnv path (contextEnv ctx) of - Just (_, Binder _ (XObj (Lst (XObj (External _) _ _ : _)) _ _)) -> Right xobj - Just (_, Binder _ (XObj (Lst (XObj (Instantiate _) _ _ : _)) _ _)) -> Right xobj - Just (_, Binder _ (XObj (Lst (XObj (Deftemplate _) _ _ : _)) _ _)) -> Right xobj - Just (_, Binder _ (XObj (Lst (XObj (Defn _) _ _ : _)) _ _)) -> Right xobj - Just (_, Binder _ (XObj (Lst (XObj Def _ _ : _)) _ _)) -> Right xobj - Just (_, Binder _ (XObj (Lst (XObj (Defalias _) _ _ : _)) _ _)) -> Right xobj - Just (_, Binder _ found) -> Right found -- use the found value - Nothing -> Right xobj -- symbols that are not found are left as-is - expandSymbol _ = error "Can't expand non-symbol in expandSymbol." + Just (_, Binder meta (XObj (Lst (XObj (External _) _ _ : _)) _ _)) -> isPrivate meta xobj + Just (_, Binder meta (XObj (Lst (XObj (Instantiate _) _ _ : _)) _ _)) -> isPrivate meta xobj + Just (_, Binder meta (XObj (Lst (XObj (Deftemplate _) _ _ : _)) _ _)) -> isPrivate meta xobj + Just (_, Binder meta (XObj (Lst (XObj (Defn _) _ _ : _)) _ _)) -> isPrivate meta xobj + Just (_, Binder meta (XObj (Lst (XObj Def _ _ : _)) _ _)) -> isPrivate meta xobj + Just (_, Binder meta (XObj (Lst (XObj (Defalias _) _ _ : _)) _ _)) -> isPrivate meta xobj + Just (_, Binder meta found) -> isPrivate meta found -- use the found value + Nothing -> return (ctx, Right xobj) -- symbols that are not found are left as-is + where + isPrivate m x = if metaIsTrue m "private" + then return (evalError ctx ("The binding: " ++ pretty sym ++ " is private; it may only be used within the module that defines it.") (info sym)) + else return (ctx, Right x) + expandSymbol _ = return (evalError ctx "Can't expand non-symbol in expandSymbol." Nothing) successiveExpand (ctx, acc) e = case acc of From 915a539d2df288b78baee1f29cbc25f6e49cacef Mon Sep 17 00:00:00 2001 From: scottolsen Date: Fri, 19 Jun 2020 14:53:00 -0400 Subject: [PATCH 3/4] Add a test for private bindings This commit adds a simple test to ensure binding privacy works as expected. --- test-for-errors/private-bindings.carp | 8 ++++++++ .../test-for-errors/private-bindings.carp.output.expected | 2 ++ 2 files changed, 10 insertions(+) create mode 100644 test-for-errors/private-bindings.carp create mode 100644 test/output/test-for-errors/private-bindings.carp.output.expected diff --git a/test-for-errors/private-bindings.carp b/test-for-errors/private-bindings.carp new file mode 100644 index 00000000..06eb40cd --- /dev/null +++ b/test-for-errors/private-bindings.carp @@ -0,0 +1,8 @@ +(Project.config "file-path-print-length" "short") + +(deftype Foo [bar Int]) +(private Foo.bar) + +(defn boo [] @(Foo.bar &(Foo.init 1))) + +(defn main [] (println* (Foo.bar &(Foo.init 1)))) diff --git a/test/output/test-for-errors/private-bindings.carp.output.expected b/test/output/test-for-errors/private-bindings.carp.output.expected new file mode 100644 index 00000000..55f72000 --- /dev/null +++ b/test/output/test-for-errors/private-bindings.carp.output.expected @@ -0,0 +1,2 @@ +private-bindings.carp:6:16 The binding: Foo.bar is private; it may only be used within the module that defines it. +private-bindings.carp:8:26 The binding: Foo.bar is private; it may only be used within the module that defines it. From 0b34f9b31a4ecc6b3c8b05aa5ac5c3833e5ca217 Mon Sep 17 00:00:00 2001 From: scottolsen Date: Fri, 19 Jun 2020 15:01:27 -0400 Subject: [PATCH 4/4] Add a valid use of a private binding to private-binding test --- test-for-errors/private-bindings.carp | 5 +++++ .../test-for-errors/private-bindings.carp.output.expected | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/test-for-errors/private-bindings.carp b/test-for-errors/private-bindings.carp index 06eb40cd..ebfad087 100644 --- a/test-for-errors/private-bindings.carp +++ b/test-for-errors/private-bindings.carp @@ -3,6 +3,11 @@ (deftype Foo [bar Int]) (private Foo.bar) +(defmodule Foo + (defn get [foo] + (Foo.bar foo)) +) + (defn boo [] @(Foo.bar &(Foo.init 1))) (defn main [] (println* (Foo.bar &(Foo.init 1)))) diff --git a/test/output/test-for-errors/private-bindings.carp.output.expected b/test/output/test-for-errors/private-bindings.carp.output.expected index 55f72000..1fd24b68 100644 --- a/test/output/test-for-errors/private-bindings.carp.output.expected +++ b/test/output/test-for-errors/private-bindings.carp.output.expected @@ -1,2 +1,2 @@ -private-bindings.carp:6:16 The binding: Foo.bar is private; it may only be used within the module that defines it. -private-bindings.carp:8:26 The binding: Foo.bar is private; it may only be used within the module that defines it. +private-bindings.carp:11:16 The binding: Foo.bar is private; it may only be used within the module that defines it. +private-bindings.carp:13:26 The binding: Foo.bar is private; it may only be used within the module that defines it.