Macro expander/Module system - build-module-name

I am implementing a macro expander for a language I am writing.
I was looking at https://github.com/mflatt/expander (in the demi branch), in module-path.rkt. There is a function build-module-name:

;; Build a submodule name given an enclosing module name, if cany
(define (build-module-name name ; a symbol
                           enclosing-module-name ; #f => no enclosing module
                           #:original [orig-name name]) ; for error reporting
  (cond
    [(not enclosing-module-name) name]
    [(symbol? enclosing-module-name) (list enclosing-module-name name)]
    [(equal? name "..")
     (cond
       [(symbol? enclosing-module-name)
        (error "too many \"..\"s:" orig-name)]
       [(= 2 (length enclosing-module-name)) (car enclosing-module-name)]
       [else (drop-right enclosing-module-name 1)])]
    [else (append enclosing-module-name (list name))]))

In the third case of the outer cond, in the inner cond, the first case should not happen because we already checked the same condition in the second case of the cond.
I then looked at the main racket repo in the expander in module-path.rkt, and it does the same thing.
I think the cases of the outer cond should be reordered like this:

;; Build a submodule name given an enclosing module name, if cany
(define (build-module-name name ; a symbol
                           enclosing-module-name ; #f => no enclosing module
                           #:original [orig-name name]) ; for error reporting
  (cond
    [(equal? name "..")
     (cond
       [(symbol? enclosing-module-name)
        (error "too many \"..\"s:" orig-name)]
       [(= 2 (length enclosing-module-name)) (car enclosing-module-name)]
       [else (drop-right enclosing-module-name 1)])]
    [(not enclosing-module-name) name]
    [(symbol? enclosing-module-name) (list enclosing-module-name name)]
    [else (append enclosing-module-name (list name))]))

I would maybe even change the condition of the first case of the inner cond to (or (not enclosing-module-name) (symbol? enclosing-module-name))

I do not know if there is some invariant I am missing, or if this is the right place to discuss this.

1 Like

Thanks for the observation and repair! I've made your changes in the "demi" branch and propagated the change to the full Racket expander — which turns out not to reach the broken conditions due to the limited way that build-module-name is used there, but still.