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

@@ -1,5 +1,14 @@
 2016-03-23  Shiro Kawai  <shiro@acm.org>
 
+	* src/load.c (Scm_Require): 'require' now loads a file into
+	  a sealed module #<module gauche.require-base>.  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.

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: #<module gauche.require-base>
+@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

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: #<module gauche.require-base>'', 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: #<module gauche.require-base>''というエラーが出たら、
+@code{use}したファイルがモジュール指定を持っていないということです。
+詳しくは@ref{Require and provide}を参照してください。
+@c COMMON
 @end defmac
 
 @node Module inheritance, Module introspection, Using modules, Modules

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 #<module gauche>) 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 #<module gauche>
-   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 #<module gauche> 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 #<module gauche> 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 #<module gauche>.
+
+   As of 0.9.5, we use an immutable module #<module gauche.require-base>
+   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);

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")