From 3b7dacd029e4ca5ca39e1d8d07d0c1626329c29f Mon Sep 17 00:00:00 2001 From: Shiro Kawai Date: Wed, 23 Mar 2016 15:38:59 -1000 Subject: [PATCH] Use special module as a base of require See the ChangeLog and comment on Scm_Require for the detailed discussion. This change might break existing code that "happened to" work; such code should be fixed. --- ChangeLog | 9 +++++++++ doc/corelib.texi | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ doc/coresyn.texi | 35 ++++++++++++++++++++++++++++++++++ src/load.c | 41 +++++++++++++++++++++++++++++----------- test/load.scm | 20 +++++++++++--------- 5 files changed, 134 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7c2fbc157d..b012ac9405 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2016-03-23 Shiro Kawai + * src/load.c (Scm_Require): 'require' now loads a file into + a sealed module #. This guarantees + the forms like define-module or define-library to be visible + from the loaded file, and also catches when the loaded file + inadvertently inserts toplevel bindings without specifying + a module. + Note: This change may triggers an error in existing code. Such + code has just "happened to work"; they should be fixed. + * src/compile.scm (pass4): Use local environment instead of inserting new global identifiers for lifted lambdas. Inserting new global ids has an issue of where module the global binding should be inserted. diff --git a/doc/corelib.texi b/doc/corelib.texi index 29ea832ae7..610eb4d991 100644 --- a/doc/corelib.texi +++ b/doc/corelib.texi @@ -18371,6 +18371,55 @@ file. We call this @emph{autoprovide} feature. @var{feature}が自動的にprovideされます。これを @emph{autoprovide}機能と呼んでいます。 @c COMMON + +@c EN +Note that @code{require} first sets the current module to +an immutable module called @code{gauche.require-base} +and then load the file. The files +loaded by @code{require} usually have @code{define-module}/@code{select-module} +or @code{define-library} for the first thing, so you rarely notice the +@code{gauche.require-base} module. However, if the loaded file +has toplevel defines or imports (@code{use}'s) without specifying +a module, you'll get an error like the following: +@c JP +@code{require}は、ファイルをロードする前に現在のモジュールを +@code{gauche.require-base}という変更不可なモジュールにセットします。 +@code{require}されるファイルは通常、最初に@code{define-module}/@code{select-module} +か@code{define-library}フォームを持つので、この@code{gauche.require-base}モジュール +を目にすることはほとんど無いでしょう。 +ただ、もしロードされたファイルがモジュールを指定すること無くトップレベルの +変数を定義したり他のモジュールを@code{import}(@code{use})しようとした場合、 +次のようなエラーとなります。 +@c COMMON + +@example +*** ERROR: Attempted to create a binding (a) in a sealed +module: # +@end example + +@c EN +Rationale: Generally it's difficult to guarantee +when the specified file is loaded by @code{require} (because some other +module may already have required it). If we just used the caller's current +module, there would be a couple of issues: The form @code{define-module} or +@code{define-library} may not be visible from the current module, and +you can't guarantee if the toplevel defines without specifying modules +in the loaded file inserts the caller's current module, since they may +have been loaded into a different module. +It is just a bad idea to insert +toplevel definitions or to import other modules without specifying which +module you put them in. So we made them an error. +@c JP +理由: @code{require}されているファイルがどのタイミングで読まれるかを +正確にコントロールするのは難しいです(他のモジュールが既にrequireしているかも +しれないので)。もし呼び出し側の現在のモジュールをそのまま使った場合、二つの +問題が生じ得ます。(1)呼び出し側の現在のモジュールから、 +@code{define-module}や@code{define-library}が見えているとは限りません。 +(2)モジュールを指定しないトップレベル定義が呼び出し側の現在のモジュールに +定義を追加することは保証されません(既に別のモジュールへと読みこまれているかもしれません)。 +モジュールを指定しないトップレベル定義やimportを持つファイルをrequireすることは、 +単に悪いアイディアです。したがってそういう事例はエラーとすることにしました。 +@c COMMON @end defspec @defun provide feature diff --git a/doc/coresyn.texi b/doc/coresyn.texi index db1fb9d9e2..4b3a80f655 100644 --- a/doc/coresyn.texi +++ b/doc/coresyn.texi @@ -3505,6 +3505,22 @@ the reader, so those effect is contained in the file even with による大文字小文字指定はリーダによって処理されるので、 効果は@code{include}されるファイルの中に留まります。) @c COMMON +@item +@c EN +It is forbidden to the file loaded by @code{require} to insert +a toplevel binding without specifying a module. In other words, +the file you require should generally use @code{define-module}, +@code{select-module} or @code{define-library}. +@xref{Require and provide}, for further discussion. +On the other hand, @code{include} has no such restrictions. +@c JP +@code{require}からロードするファイルが、モジュールを指定せずにトップレベル +束縛を挿入することは禁じられています。別の言い方をすれば、 +@code{require}するファイルは一般的に@code{define-module}や@code{select-module}、 +あるいは@code{define-library}を使う必要があるということです。 +詳しい議論は@ref{Require and provide}を参照してください。 +一方で、@code{include}にはそういった制限はありません。 +@c COMMON @end itemize @item @code{load} @@ -4626,6 +4642,25 @@ In future, there may be some mechanism to customize this mapping. となります。これはあまりScheme風ではありませんが、便利ではあります。 将来、このマッピングルールをカスタマイズする機構が導入されるかもしれません。 @c COMMON + +@c EN +The file to be @code{use}'d must have explict module selection +to have any toplevel definitions +(usually via @code{define-module}/@code{select-module} pair +or @code{define-library}). If you get an error saying +``Attempted to create a binding in a sealed module: +module: #'', that's because the file +lacks module selection. @xref{Require and provide}, for further +discussion. +@c JP +@code{use}されるファイルがトップレベル定義を持つ場合、ファイル内でモジュールが +明示されてることが必要です (通常は@code{define-module}/@code{select-module} +や@code{define-library}が使われます)。 +もし、``Attempted to create a binding in a sealed module: +module: #''というエラーが出たら、 +@code{use}したファイルがモジュール指定を持っていないということです。 +詳しくは@ref{Require and provide}を参照してください。 +@c COMMON @end defmac @node Module inheritance, Module introspection, Using modules, Modules diff --git a/src/load.c b/src/load.c index f1181067d3..140d633f0b 100644 --- a/src/load.c +++ b/src/load.c @@ -84,6 +84,19 @@ static struct { but we may change this design in future. */ ScmInternalMutex dso_mutex; + + /* An immutable module to which 'require' loads code. + We need a base module where 'define-module' and 'define-library' are + visible in order for requiring modules using them to work, so + loading into the current module won't cut it. However, we don't + want to use a specific mutable module (such as #) as + a base, since if the required module has toplevel defines without + switching the module, it will modify the base module. + By using immutable module as a base, we can reject the latter case; + requiring a code that inserts toplevel binding without specifying + a module is simply a bad idea and shouldn't be allowed. + */ + ScmModule *require_base; } ldinfo = { (ScmGloc*)&ldinfo, }; /* trick to put ldinfo in .data section */ /* keywords used for load and load-from-port surbs */ @@ -830,20 +843,21 @@ static int do_require(ScmObj, int, ScmModule *, ScmLoadPacket *); is loaded at this time or it has already been loaded. With the same reason, it doesn't make much sense to use the current module. - Until 0.9.4 we didn't actually force 'require' to use # - as the base module. Most of the time, 'require' is called in a module - that inherits gauche, so the problem didn't appear. However, if one - requires a file from a module that doesn't inherit gauche, most likely - accidentally, he sees confusing errors---because "define-module" form - isn't recognized! - - As of 0.9.4 we fix the base module to # when require is - called. Note that autoload needs different requirements, so we have - a subroutine do_require that takes the desired base module. + On 0.9.4 we always set the base module to # to do require, + so that we can guarantee the forms like define-module or define-library + to be visible from the loaded module (if we use the caller's current + module it is not guaranteed.) However, it had an unexpected side + effect: If the loaded module inserts toplevel definitions or imports + other modules without first setting its own module, it actually + modifies #. + + As of 0.9.5, we use an immutable module # + as the base module. Since it is immutable, any toplevel definitions + or imports without first switching modules are rejected. */ int Scm_Require(ScmObj feature, int flags, ScmLoadPacket *packet) { - return do_require(feature, flags, Scm_GaucheModule(), packet); + return do_require(feature, flags, ldinfo.require_base, packet); } int do_require(ScmObj feature, int flags, ScmModule *base_mod, @@ -1257,6 +1271,11 @@ void Scm__InitLoad(void) ldinfo.dso_table = SCM_HASH_TABLE(Scm_MakeHashTableSimple(SCM_HASH_STRING,0)); ldinfo.dso_prelinked = SCM_NIL; + ldinfo.require_base = + SCM_MODULE(Scm_MakeModule(SCM_SYMBOL(SCM_INTERN("gauche.require-base")), + FALSE)); + Scm_ModuleSeal(ldinfo.require_base); + #define PARAM_INIT(var, name, val) Scm_DefinePrimitiveParameter(m, name, val, &ldinfo.var) PARAM_INIT(load_history, "current-load-history", SCM_NIL); PARAM_INIT(load_next, "current-load-next", SCM_NIL); diff --git a/test/load.scm b/test/load.scm index acedbc6788..82f4b5efe9 100644 --- a/test/load.scm +++ b/test/load.scm @@ -66,15 +66,17 @@ (test "reload after error" 1 (^[] - (with-error-handler - (^e #t) - (^[] (eval '(require "test.o/d") (interaction-environment)))) - (with-output-to-file "test.o/d.scm" - (^[] - (write '(define z 1)) - (write '(provide "tset.o/d")))) - (eval '(require "test.o/d") (interaction-environment)) - (eval 'z (interaction-environment)))) + (let ((m (make-module 'reload-after-error))) + (with-error-handler + (^e #t) + (^[] (eval '(require "test.o/d") (interaction-environment)))) + (with-output-to-file "test.o/d.scm" + (^[] + (write '(select-module reload-after-error)) + (write '(define z 1)) + (write '(provide "tset.o/d")))) + (eval '(require "test.o/d") (interaction-environment)) + (eval 'z m)))) ;; :environment arg ------------------------------------- (test-section "load environment")