Distinguish quoted and unquoted variables

Now only quoted strings support concatenation of text and a split-variable.

CHANGES.md

@@ -1,6 +1,11 @@
 1.0+beta18 (14/02/2018)
 -----------------------
 
+- Let the parser distinguish quoted strings from atoms.  This makes
+  possible to use "${v}" to concatenate the list of values provided by
+  a split-variable.  Concatenating split-variables with text is also
+  now required to be quoted.
+
 - Split calls to ocamldep. Before ocamldep would be called once per
   `library`/`executables` stanza. Now it is called once per file
   (#486)

bootstrap.ml

@@ -33,7 +33,7 @@ let dirs =
   ; "src/fiber"        , Some "Fiber"
   ; "src/xdg"          , Some "Xdg"
   ; "vendor/boot"      , None
-  ; "vendor/usexp/src" , Some "Usexp"
+  ; "src/usexp"        , Some "Usexp"
   ; "src"              , None
   ]
 

doc/jbuild.rst

@@ -765,14 +765,13 @@ you have to quote the variable as in:
 
 .. code:: scheme
 
-    (run foo "${^} ")
+    (run foo "${^}")
 
-(for now the final space is necessary)
 which is equivalent to the following shell command:
 
 .. code:: shell
 
-    $ foo "a b "
+    $ foo "a b"
 
 (the items of the list are concatenated with space).
 Note that, since ``${^}`` is a list of items, the first one may be

doc/project-layout-specification.rst

@@ -26,29 +26,80 @@ Metadata format
 ===============
 
 Most configuration files read by Jbuilder are using the S-expression
-syntax, which is very simple. Everything is either an atom or a list.
-The exact specification of S-expressions is described in the
-documentation of the `parsexp <https://github.com/janestreet/parsexp>`__
-library.
+syntax, which is very simple.  It is described below.
 
-In a nutshell, the syntax is as follows:
+Note that the format is completely static. However you can do
+meta-programming on jbuilds files by writing them in :ref:`ocaml-syntax`.
 
-- atoms that do no contain special characters are simply written as
-  is. For instance: ``foo``, ``bar`` are valid atomic S-expressions
 
-- atoms containing special characters or spaces must be quoted using
-  the syntax ``"..."``: ``"foo bar\n"``
+Lexical conventions of s-expressions
+------------------------------------
 
-- lists are formed by surrounding a sequence of S-expressions separated
-  by spaces with parentheses: ``(a b (c d))``
+Whitespace, which consists of space, newline, horizontal tab, and form
+feed, is ignored unless within an OCaml-string, where it is treated
+according to OCaml-conventions.  The left parenthesis opens a new
+list, the right one closes it.  Lists can be empty.
 
-- single-line comments are introduced with the ``;`` character and may
-  appear anywhere except in the middle of a quoted atom
+The double quote denotes the beginning and end of a string using
+similar lexing conventions to the ones of OCaml (see the OCaml-manual
+for details).  Differences are:
 
-- block comment are enclosed by ``#|`` and ``|#`` and can be nested
+- octal escape sequences (``\o123``) are not supported;
+- backslash that's not a part of any escape sequence is kept as it is
+  instead of resulting in parse error;
+- a backslash followed by a space does not form an escape sequence, so
+  it’s interpreted as is, while it is interpreted as just a space by
+  OCaml.
+
+All characters other than double quotes, left- and right parentheses,
+whitespace, carriage return, and comment-introducing characters or
+sequences (see next paragraph) are considered part of a contiguous
+string.
+
+Comments
+--------
+
+There are three kinds of comments:
+
+- line comments are introduced with ``;``, and end at the newline;
+- sexp comments are introduced with ``#;``, and end at the end of the
+  following s-expression;
+- block comments are introduced with ``#|`` and end with ``|#``.
+  These can be nested, and double-quotes within them must be balanced
+  and be lexically correct OCaml strings.
+
+Grammar of s-expressions
+------------------------
+
+S-expressions are either sequences of non-whitespace characters
+(= atoms), doubly quoted strings or lists. The lists can recursively
+contain further s-expressions or be empty, and must be balanced,
+i.e. parentheses must match.
+
+Examples
+--------
+
+::
+
+    this_is_an_atom_123'&^%!  ; this is a comment
+    "another atom in an OCaml-string \"string in a string\" \123"
+    
+    ; empty list follows below
+    ()
+    
+    ; a more complex example
+    (
+      (
+        list in a list  ; comment within a list
+        (list in a list in a list)
+        42 is the answer to all questions
+        #; (this S-expression
+             (has been commented out)
+           )
+        #| Block comments #| can be "nested" |# |#
+      )
+    )
 
-Note that the format is completely static. However you can do
-meta-programming on jbuilds files by writing them in :ref:`ocaml-syntax`.
 
 .. _opam-files:
 

src/action.ml

@@ -180,7 +180,7 @@ module Prog = struct
 
   let sexp_of_t = function
     | Ok s -> Path.sexp_of_t s
-    | Error (e : Not_found.t) -> Sexp.To_sexp.string e.program
+    | Error (e : Not_found.t) -> Sexp.To_sexp.atom e.program
 end
 
 module type Ast = Action_intf.Ast
@@ -192,7 +192,7 @@ module rec Ast : Ast = Ast
 module String_with_sexp = struct
   type t = string
   let t = Sexp.Of_sexp.string
-  let sexp_of_t = Sexp.To_sexp.string
+  let sexp_of_t = Sexp.To_sexp.atom
 end
 
 include Make_ast
@@ -272,38 +272,44 @@ module Var_expansion = struct
     | Paths   of Path.t list * Concat_or_split.t
     | Strings of string list * Concat_or_split.t
 
+  let is_multivalued = function
+    | Paths (_, Split) | Strings (_, Split) -> true
+    | Paths (_, Concat) | Strings (_, Concat) -> false
+
+  type context = Path.t (* For String_with_vars.Expand_to *)
+
   let concat = function
     | [s] -> s
     | l -> String.concat ~sep:" " l
 
   let string_of_path ~dir p = Path.reach ~from:dir p
-  let path_of_string ~dir s = Path.relative dir s
+  let path_of_string dir s = Path.relative dir s
 
-  let to_strings ~dir = function
+  let to_strings dir = function
     | Strings (l, Split ) -> l
     | Strings (l, Concat) -> [concat l]
     | Paths   (l, Split ) -> List.map l ~f:(string_of_path ~dir)
     | Paths   (l, Concat) -> [concat (List.map l ~f:(string_of_path ~dir))]
 
-  let to_string ~dir = function
+  let to_string (dir: context) = function
     | Strings (l, _) -> concat l
     | Paths   (l, _) -> concat (List.map l ~f:(string_of_path ~dir))
 
-  let to_path ~dir = function
-    | Strings (l, _) -> path_of_string ~dir (concat l)
+  let to_path dir = function
+    | Strings (l, _) -> path_of_string dir (concat l)
     | Paths ([p], _) -> p
     | Paths (l,   _) ->
-      path_of_string ~dir (concat (List.map l ~f:(string_of_path ~dir)))
+      path_of_string dir (concat (List.map l ~f:(string_of_path ~dir)))
 
-  let to_prog_and_args ~dir exp : Unresolved.Program.t * string list =
+  let to_prog_and_args dir exp : Unresolved.Program.t * string list =
     let module P = Unresolved.Program in
     match exp with
     | Paths   ([p], _) -> (This p, [])
     | Strings ([s], _) -> (P.of_string ~dir s, [])
     | Paths ([], _) | Strings ([], _) -> (Search "", [])
     | Paths (l, Concat) ->
       (This
-         (path_of_string ~dir
+         (path_of_string dir
             (concat (List.map l ~f:(string_of_path ~dir)))),
        [])
     | Strings (l, Concat) ->
@@ -315,6 +321,7 @@ module Var_expansion = struct
 end
 
 module VE = Var_expansion
+module To_VE = String_with_vars.Expand_to(VE)
 module SW = String_with_vars
 
 module Unexpanded = struct
@@ -328,7 +335,7 @@ module Unexpanded = struct
 
   let t sexp =
     match sexp with
-    | Atom _ ->
+    | Atom _ | Quoted_string _ ->
       of_sexp_errorf sexp
         "if you meant for this to be executed with bash, write (bash \"...\") instead"
     | List _ -> t sexp
@@ -352,28 +359,23 @@ module Unexpanded = struct
     include Past
 
     module E = struct
-      let string ~dir ~f = function
-        | Inl x -> x
-        | Inr template ->
-          SW.expand template ~f:(fun loc var ->
-            match f loc var with
-            | None   -> None
-            | Some e -> Some (VE.to_string ~dir e))
-
       let expand ~generic ~special ~map ~dir ~f = function
         | Inl x -> map x
-        | Inr template as x ->
-          match SW.just_a_var template with
-          | None -> generic ~dir (string ~dir ~f x)
-          | Some var ->
-            match f (SW.loc template) var with
-            | None   -> generic ~dir (SW.to_string template)
-            | Some e -> special ~dir e
+        | Inr template ->
+           match To_VE.expand dir template ~f with
+           | Inl e -> special dir e
+           | Inr s -> generic dir s
       [@@inlined always]
 
+      let string ~dir ~f x =
+        expand ~dir ~f x
+          ~generic:(fun _dir x -> x)
+          ~special:VE.to_string
+          ~map:(fun x -> x)
+
       let strings ~dir ~f x =
         expand ~dir ~f x
-          ~generic:(fun ~dir:_ x -> [x])
+          ~generic:(fun _dir x -> [x])
           ~special:VE.to_strings
           ~map:(fun x -> [x])
 
@@ -385,7 +387,7 @@ module Unexpanded = struct
 
       let prog_and_args ~dir ~f x =
         expand ~dir ~f x
-          ~generic:(fun ~dir:_ s -> (Program.of_string ~dir s, []))
+          ~generic:(fun _dir s -> (Program.of_string ~dir s, []))
           ~special:VE.to_prog_and_args
           ~map:(fun x -> (x, []))
     end
@@ -445,27 +447,20 @@ module Unexpanded = struct
   end
 
   module E = struct
-    let string ~dir ~f template =
-      SW.partial_expand template ~f:(fun loc var ->
-        match f loc var with
-        | None   -> None
-        | Some e -> Some (VE.to_string ~dir e))
-
     let expand ~generic ~special ~dir ~f template =
-      match SW.just_a_var template with
-      | None -> begin
-          match string ~dir ~f template with
-          | Inl x -> Inl (generic ~dir x)
-          | Inr _ as x -> x
-        end
-      | Some var ->
-        match f (SW.loc template) var with
-        | None   -> Inr template
-        | Some e -> Inl (special ~dir e)
+      match To_VE.partial_expand dir template ~f with
+      | Inl (Inl e) -> Inl(special dir e)
+      | Inl (Inr s) -> Inl(generic dir s)
+      | Inr _ as x -> x
+
+    let string ~dir ~f x =
+      expand ~dir ~f x
+        ~generic:(fun _dir x -> x)
+        ~special:VE.to_string
 
     let strings ~dir ~f x =
       expand ~dir ~f x
-        ~generic:(fun ~dir:_ x -> [x])
+        ~generic:(fun _dir x -> [x])
         ~special:VE.to_strings
 
     let path ~dir ~f x =
@@ -475,7 +470,7 @@ module Unexpanded = struct
 
     let prog_and_args ~dir ~f x =
       expand ~dir ~f x
-        ~generic:(fun ~dir s -> (Unresolved.Program.of_string ~dir s, []))
+        ~generic:(fun dir s -> (Unresolved.Program.of_string ~dir s, []))
         ~special:VE.to_prog_and_args
   end
 

src/action.mli

@@ -3,13 +3,18 @@ open! Import
 module Var_expansion : sig
   module Concat_or_split : sig
     type t =
-      | Concat (* default *)
-      | Split  (* the variable is a "split" list of items *)
+      | Concat (** default *)
+      | Split  (** the variable is a "split" list of items *)
   end
 
   type t =
     | Paths   of Path.t list * Concat_or_split.t
     | Strings of string list * Concat_or_split.t
+
+  val to_string : Path.t -> t -> string
+  (** [to_string dir v] convert the variable expansion to a string.
+     If it is a path, the corresponding string will be relative to
+     [dir]. *)
 end
 
 module Outputs : module type of struct include Action_intf.Outputs end

src/context.ml

@@ -13,8 +13,8 @@ module Kind = struct
   let sexp_of_t : t -> Sexp.t = function
     | Default -> Atom "default"
     | Opam o  ->
-      Sexp.To_sexp.(record [ "root"  , string o.root
-                           ; "switch", string o.switch
+      Sexp.To_sexp.(record [ "root"  , atom o.root
+                           ; "switch", atom o.switch
                            ])
 end
 
@@ -92,10 +92,10 @@ let sexp_of_t t =
   let open Sexp.To_sexp in
   let path = Path.sexp_of_t in
   record
-    [ "name", string t.name
+    [ "name", atom t.name
     ; "kind", Kind.sexp_of_t t.kind
     ; "merlin", bool t.merlin
-    ; "for_host", option string (Option.map t.for_host ~f:(fun t -> t.name))
+    ; "for_host", option atom (Option.map t.for_host ~f:(fun t -> t.name))
     ; "build_dir", path t.build_dir
     ; "toplevel_path", option path t.toplevel_path
     ; "ocaml_bin", path t.ocaml_bin
@@ -104,13 +104,13 @@ let sexp_of_t t =
     ; "ocamlopt", option path t.ocamlopt
     ; "ocamldep", path t.ocamldep
     ; "ocamlmklib", path t.ocamlmklib
-    ; "env", list (pair string string) (Env_var_map.bindings t.env_extra)
+    ; "env", list (pair atom atom) (Env_var_map.bindings t.env_extra)
     ; "findlib_path", list path (Findlib.path t.findlib)
     ; "arch_sixtyfour", bool t.arch_sixtyfour
     ; "natdynlink_supported", bool t.natdynlink_supported
-    ; "opam_vars", string_hashtbl string t.opam_var_cache
-    ; "ocamlc_config", list (pair string string) t.ocamlc_config
-    ; "which", string_hashtbl (option path) t.which_cache
+    ; "opam_vars", atom_hashtbl atom t.opam_var_cache
+    ; "ocamlc_config", list (pair atom atom) t.ocamlc_config
+    ; "which", atom_hashtbl (option path) t.which_cache
     ]
 
 let compare a b = compare a.name b.name

src/gen_rules.ml

@@ -84,7 +84,7 @@ module Gen(P : Params) = struct
              \n\
              \nThis will become an error in the future."
             (Sexp.to_string (List [ Atom "modules_without_implementation"
-                                  ; Sexp.To_sexp.(list string) should_be_listed
+                                  ; Sexp.To_sexp.(list atom) should_be_listed
                                   ]))
         | Some loc ->
           Loc.warn loc