bug#69387: 30.0.50; A string shouldn't be both a docstring and a return value

["Mattias Engdegård" <mattias.engdegard@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

> I agree. I'll make a patch for the warning, just so that we can see what it would look like.

Here, belatedly. It includes the lambda string change. The warning includes `defun` and `defmacro` but not plain lambdas.

Removing doc strings from ambiguous lambdas didn't break anything, as expected. I think the warning makes sense, because it does expose some mistakes where doc strings were returned but shouldn't be, or returned strings also exposed as (nonsensical) doc strings.

There are quite a few cl-defmethod forms with this syndrome but those definitions never were ambiguous (the strings  are always a return value, never doc string).


[-- Attachment #2: docstring-result-w.diff --]
[-- Type: application/octet-stream, Size: 2117 bytes --]

diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el
index cc176821026..42a25bcc0aa 100644
--- a/lisp/emacs-lisp/byte-run.el
+++ b/lisp/emacs-lisp/byte-run.el
@@ -263,6 +263,11 @@ 'byte-run--set-no-font-lock-keyword
 (defalias 'byte-run--parse-body
   #'(lambda (body allow-interactive)
       "Decompose BODY into (DOCSTRING DECLARE INTERACTIVE BODY-REST WARNINGS)."
+      (if (and (null (cdr body)) (stringp (car body)))
+          (list nil nil nil body
+                (list (macroexp-warn-and-return
+                       "Single string treated as return value, not doc string"
+                       nil nil t body)))
         (let* ((top body)
                (docstring nil)
                (declare-form nil)
@@ -305,7 +310,7 @@ 'byte-run--parse-body
                         (t (setq interactive-form form)))
                        t))))
             (setq body (cdr body)))
-        (list docstring declare-form interactive-form body warnings))))
+          (list docstring declare-form interactive-form body warnings)))))
 
 (defalias 'byte-run--parse-declarations
   #'(lambda (name arglist clauses construct declarations-alist)
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index c3355eedd75..cf0e6d600dd 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -3061,12 +3061,11 @@ byte-compile-lambda
 	  (append (if (not lexical-binding) arglistvars)
                   byte-compile-bound-variables))
 	 (body (cdr (cdr fun)))
-	 (doc (if (stringp (car body))
+         ;; Treat a final string literal as a value, not a doc string.
+	 (doc (if (and (cdr body) (stringp (car body)))
                   (prog1 (car body)
-                    ;; Discard the doc string from the body
-                    ;; unless it is the last element of the body.
-                    (if (cdr body)
-                        (setq body (cdr body))))))
+                    ;; Discard the doc string from the body.
+                    (setq body (cdr body)))))
 	 (int (assq 'interactive body))
          command-modes)
     (when lexical-binding