From a2182d79eac73edfab614b6c1b3881938c46e058 Mon Sep 17 00:00:00 2001
From: Kaz Kylheku <kaz@kylheku.com>
Date: Mon, 6 Jan 2020 11:17:53 -0800
Subject: builtin redefinition: better diagnostic message.

The motivation here is that if we define, say, a macro whose
name is the same as a built-in function, we get a warning
which misleadingly uses the word "redefining".

* eval.c (builtin_reject_test): Add a new parameter which
indicates what kind of binding is being defined. This has the
same values as the builtin hash. If the builtin hash reports
that the symbol is a builtin, we can issue one of two
diagnostic messages based on whether the one being defined is
of the same kind or noto.
(expand_macrolet): Pass the defmacro symbol to
builtin_reject_test, since macros are being defined.
(expand_fbind_vars): Pass the defun symbol symbol to
builtin_reject_test.
(do_expand): In the defun/defmacro case, pass the operator in
question to builtin_reject_test.
---
 eval.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/eval.c b/eval.c
index 6d11cfef..97e9c5ee 100644
--- a/eval.c
+++ b/eval.c
@@ -2143,7 +2143,7 @@ static val maybe_quote(val form)
   return cons(quote_s, cons(form, nil));
 }
 
-static void builtin_reject_test(val op, val sym, val form)
+static void builtin_reject_test(val op, val sym, val form, val def_kind)
 {
     val builtin_kind = gethash(builtin, sym);
     val is_operator = gethash(op_table, sym);
@@ -2160,8 +2160,12 @@ static void builtin_reject_test(val op, val sym, val form)
     } else if (sym == expr_s || sym == var_s) {
       /* empty */
     } else if (builtin_kind) {
-      eval_warn(form, lit("~s: redefining ~s, which is a built-in ~s"),
-                op, sym, builtin_kind, nao);
+      if (builtin_kind == def_kind)
+        eval_warn(form, lit("~s: redefining ~s, which is a built-in ~s"),
+                  op, sym, builtin_kind, nao);
+      else
+        eval_warn(form, lit("~s: defining ~s, which is also a built-in ~s"),
+                  op, sym, builtin_kind, nao);
     } else if (is_operator) {
       eval_warn(form, lit("~s: redefining ~s, which is a built-in operator"),
                 op, sym, nao);
@@ -2213,7 +2217,7 @@ static val expand_macrolet(val form, val menv)
     val macro_out = expand_forms(macro_ex, new_menv);
     val block = rlcp_tree(cons(block_s, cons(name, macro_out)), macro_ex);
 
-    builtin_reject_test(op, name, form);
+    builtin_reject_test(op, name, form, defmacro_s);
 
     /* We store the macrolet in the same form as a top level defmacro,
      * so they can be treated uniformly. The nil at the head of the
@@ -3679,7 +3683,7 @@ static val expand_fbind_vars(val vars, val menv, val form)
     val rest_vars_ex = rlcp(expand_fbind_vars(rest_vars, menv, form),
                             rest_vars);
 
-    builtin_reject_test(car(form), var, form);
+    builtin_reject_test(car(form), var, form, defun_s);
 
     if (init == init_ex && rest_vars == rest_vars_ex)
       return vars;
@@ -4688,7 +4692,7 @@ again:
       val name = second(form);
       val params = third(form);
 
-      builtin_reject_test(sym, name, form);
+      builtin_reject_test(sym, name, form, sym);
 
       if (sym == defun_s)
         uw_register_tentative_def(cons(fun_s, name));
-- 
cgit v1.2.3