all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Unused/obsolete `last-code-conversion-error'?
@ 2014-08-27 13:09 Dmitry Antipov
  2014-08-27 14:53 ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Antipov @ 2014-08-27 13:09 UTC (permalink / raw)
  To: Emacs development discussions

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

AFAICS `last-code-conversion-error' is untouched and unused since 2009
(probably it was never really used), and maintaining it just adds some
overhead to important coding.c routines.  OK to install something like this?

Dmitry

[-- Attachment #2: last_code_conversion_error.patch --]
[-- Type: text/x-patch, Size: 9022 bytes --]

=== modified file 'lisp/international/mule.el'
--- lisp/international/mule.el	2014-01-01 07:43:34 +0000
+++ lisp/international/mule.el	2014-08-27 13:01:10 +0000
@@ -955,6 +955,10 @@
   "It exists just for backward compatibility, and the value is always nil.")
 (make-obsolete-variable 'char-coding-system-table nil "23.1")
 
+(defconst last-code-conversion-error nil
+  "It exists just for backward compatibility, and the value is always nil.")
+(make-obsolete-variable 'last-code-conversion-error nil "24.4")
+
 (defun transform-make-coding-system-args (name type &optional doc-string props)
   "For internal use only.
 Transform XEmacs style args for `make-coding-system' to Emacs style.

=== modified file 'src/coding.c'
--- src/coding.c	2014-08-11 00:59:34 +0000
+++ src/coding.c	2014-08-27 12:58:32 +0000
@@ -326,8 +326,6 @@
 Lisp_Object Qstart_process, Qopen_network_stream;
 static Lisp_Object Qtarget_idx;
 
-static Lisp_Object Qinsufficient_source, Qinvalid_source, Qinterrupted;
-
 /* If a symbol has this property, evaluate the value to define the
    symbol as a coding system.  */
 static Lisp_Object Qcoding_system_define_form;
@@ -704,8 +702,8 @@
     if (src == src_end)					\
       {							\
 	if (src_base < src)				\
-	  record_conversion_result			\
-	    (coding, CODING_RESULT_INSUFFICIENT_SRC);	\
+	  coding->result				\
+	    = CODING_RESULT_INSUFFICIENT_SRC;		\
 	goto no_more_source;				\
       }							\
     c = *src++;						\
@@ -717,8 +715,8 @@
 	  {						\
 	    src--;					\
 	    c = - string_char (src, &src, NULL);	\
-	    record_conversion_result			\
-	      (coding, CODING_RESULT_INVALID_SRC);	\
+	    coding->result				\
+	      = CODING_RESULT_INVALID_SRC;		\
 	  }						\
       }							\
     consumed_chars++;					\
@@ -847,35 +845,6 @@
     EMIT_TWO_BYTES (c3, c4);			\
   } while (0)
 
-
-static void
-record_conversion_result (struct coding_system *coding,
-			  enum coding_result_code result)
-{
-  coding->result = result;
-  switch (result)
-    {
-    case CODING_RESULT_INSUFFICIENT_SRC:
-      Vlast_code_conversion_error = Qinsufficient_source;
-      break;
-    case CODING_RESULT_INVALID_SRC:
-      Vlast_code_conversion_error = Qinvalid_source;
-      break;
-    case CODING_RESULT_INTERRUPT:
-      Vlast_code_conversion_error = Qinterrupted;
-      break;
-    case CODING_RESULT_INSUFFICIENT_DST:
-      /* Don't record this error in Vlast_code_conversion_error
-	 because it happens just temporarily and is resolved when the
-	 whole conversion is finished.  */
-      break;
-    case CODING_RESULT_SUCCESS:
-      break;
-    default:
-      Vlast_code_conversion_error = intern ("Unknown error");
-    }
-}
-
 /* These wrapper macros are used to preserve validity of pointers into
    buffer text across calls to decode_char, encode_char, etc, which
    could cause relocation of buffers if it loads a charset map,
@@ -1551,7 +1520,7 @@
 	}
       produced_chars = dst - (coding->destination + coding->produced);
     }
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
   coding->produced_char += produced_chars;
   coding->produced = dst - coding->destination;
   return 0;
@@ -1825,7 +1794,7 @@
 	    EMIT_FOUR_BYTES (c1 & 0xFF, c1 >> 8, c2 & 0xFF, c2 >> 8);
 	}
     }
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
   coding->produced = dst - coding->destination;
   coding->produced_char += produced_chars;
   return 0;
@@ -2739,7 +2708,7 @@
 	    }
 	}
     }
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
   coding->produced_char += produced_chars;
   coding->produced = dst - coding->destination;
   return 0;
@@ -4576,7 +4545,7 @@
       ASSURE_DESTINATION (safe_room);
       ENCODE_RESET_PLANE_AND_REGISTER ();
     }
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
   CODING_ISO_BOL (coding) = bol_designation;
   coding->produced_char += produced_chars;
   coding->produced = dst - coding->destination;
@@ -5032,7 +5001,7 @@
 	    EMIT_ONE_ASCII_BYTE (code & 0x7F);
 	}
     }
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
   coding->produced_char += produced_chars;
   coding->produced = dst - coding->destination;
   return 0;
@@ -5104,7 +5073,7 @@
 	    EMIT_ONE_ASCII_BYTE (code & 0x7F);
 	}
     }
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
   coding->produced_char += produced_chars;
   coding->produced = dst - coding->destination;
   return 0;
@@ -5218,17 +5187,17 @@
   switch (ccl->status)
     {
     case CCL_STAT_SUSPEND_BY_SRC:
-      record_conversion_result (coding, CODING_RESULT_INSUFFICIENT_SRC);
+      coding->result = CODING_RESULT_INSUFFICIENT_SRC;
       break;
     case CCL_STAT_SUSPEND_BY_DST:
-      record_conversion_result (coding, CODING_RESULT_INSUFFICIENT_DST);
+      coding->result = CODING_RESULT_INSUFFICIENT_DST;
       break;
     case CCL_STAT_QUIT:
     case CCL_STAT_INVALID_CMD:
-      record_conversion_result (coding, CODING_RESULT_INTERRUPT);
+      coding->result = CODING_RESULT_INTERRUPT;
       break;
     default:
-      record_conversion_result (coding, CODING_RESULT_SUCCESS);
+      coding->result = CODING_RESULT_SUCCESS;
       break;
     }
   coding->consumed_char += consumed_chars;
@@ -5289,17 +5258,17 @@
   switch (ccl->status)
     {
     case CCL_STAT_SUSPEND_BY_SRC:
-      record_conversion_result (coding, CODING_RESULT_INSUFFICIENT_SRC);
+      coding->result = CODING_RESULT_INSUFFICIENT_SRC;
       break;
     case CCL_STAT_SUSPEND_BY_DST:
-      record_conversion_result (coding, CODING_RESULT_INSUFFICIENT_DST);
+      coding->result = CODING_RESULT_INSUFFICIENT_DST;
       break;
     case CCL_STAT_QUIT:
     case CCL_STAT_INVALID_CMD:
-      record_conversion_result (coding, CODING_RESULT_INTERRUPT);
+      coding->result = CODING_RESULT_INTERRUPT;
       break;
     default:
-      record_conversion_result (coding, CODING_RESULT_SUCCESS);
+      coding->result = CODING_RESULT_SUCCESS;
       break;
     }
 
@@ -5326,10 +5295,10 @@
     {
       coding->consumed_char--;
       coding->consumed--;
-      record_conversion_result (coding, CODING_RESULT_INSUFFICIENT_SRC);
+      coding->result = CODING_RESULT_INSUFFICIENT_SRC;
     }
   else
-    record_conversion_result (coding, CODING_RESULT_SUCCESS);
+    coding->result = CODING_RESULT_SUCCESS;
 }
 
 static bool
@@ -5406,7 +5375,7 @@
 	}
       produced_chars = dst - (coding->destination + coding->produced);
     }
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
   coding->produced_char += produced_chars;
   coding->produced = dst - coding->destination;
   return 0;
@@ -5704,7 +5673,7 @@
 	}
     }
 
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
   coding->produced_char += produced_chars;
   coding->produced = dst - coding->destination;
   return 0;
@@ -7366,7 +7335,7 @@
   coding->consumed = coding->consumed_char = 0;
   coding->produced = coding->produced_char = 0;
   coding->chars_at_source = 0;
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
 
   ALLOC_CONVERSION_WORK_AREA (coding, coding->src_bytes);
 
@@ -7761,7 +7730,7 @@
 
   coding->consumed = coding->consumed_char = 0;
   coding->produced = coding->produced_char = 0;
-  record_conversion_result (coding, CODING_RESULT_SUCCESS);
+  coding->result = CODING_RESULT_SUCCESS;
 
   ALLOC_CONVERSION_WORK_AREA (coding, coding->src_chars);
 
@@ -10944,9 +10913,6 @@
   ASET (Vcoding_category_table, coding_category_undecided,
 	intern_c_string ("coding-category-undecided"));
 
-  DEFSYM (Qinsufficient_source, "insufficient-source");
-  DEFSYM (Qinvalid_source, "invalid-source");
-  DEFSYM (Qinterrupted, "interrupted");
   DEFSYM (Qcoding_system_define_form, "coding-system-define-form");
 
   defsubr (&Scoding_system_p);
@@ -11047,23 +11013,6 @@
 Coding system used in the latest file or process I/O.  */);
   Vlast_coding_system_used = Qnil;
 
-  DEFVAR_LISP ("last-code-conversion-error", Vlast_code_conversion_error,
-	       doc: /*
-Error status of the last code conversion.
-
-When an error was detected in the last code conversion, this variable
-is set to one of the following symbols.
-  `insufficient-source'
-  `inconsistent-eol'
-  `invalid-source'
-  `interrupted'
-  `insufficient-memory'
-When no error was detected, the value doesn't change.  So, to check
-the error status of a code conversion by this variable, you must
-explicitly set this variable to nil before performing code
-conversion.  */);
-  Vlast_code_conversion_error = Qnil;
-
   DEFVAR_BOOL ("inhibit-eol-conversion", inhibit_eol_conversion,
 	       doc: /*
 *Non-nil means always inhibit code conversion of end-of-line format.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Unused/obsolete `last-code-conversion-error'?
  2014-08-27 13:09 Unused/obsolete `last-code-conversion-error'? Dmitry Antipov
@ 2014-08-27 14:53 ` Eli Zaretskii
  2014-08-27 16:21   ` Dmitry Antipov
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2014-08-27 14:53 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: emacs-devel

> Date: Wed, 27 Aug 2014 17:09:03 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> 
> AFAICS `last-code-conversion-error' is untouched and unused since 2009

It is assigned values in coding.c, so "untouched" is inaccurate.

> OK to install something like this?

Why is this an improvement?



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Unused/obsolete `last-code-conversion-error'?
  2014-08-27 14:53 ` Eli Zaretskii
@ 2014-08-27 16:21   ` Dmitry Antipov
  2014-08-27 16:54     ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Antipov @ 2014-08-27 16:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On 08/27/2014 06:53 PM, Eli Zaretskii wrote:

> It is assigned values in coding.c, so "untouched" is inaccurate.

I mean "last change around it is dated back to 2009".

> Why is this an improvement?

Because it is just another set-but-never-used variable.

Dmitry




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Unused/obsolete `last-code-conversion-error'?
  2014-08-27 16:21   ` Dmitry Antipov
@ 2014-08-27 16:54     ` Eli Zaretskii
  2014-08-27 17:21       ` Dmitry Antipov
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2014-08-27 16:54 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: emacs-devel

> Date: Wed, 27 Aug 2014 20:21:51 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> On 08/27/2014 06:53 PM, Eli Zaretskii wrote:
> 
> > It is assigned values in coding.c, so "untouched" is inaccurate.
> 
> I mean "last change around it is dated back to 2009".
> 
> > Why is this an improvement?
> 
> Because it is just another set-but-never-used variable.

But your change still assigns values to variables, so I see no real
difference.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Unused/obsolete `last-code-conversion-error'?
  2014-08-27 16:54     ` Eli Zaretskii
@ 2014-08-27 17:21       ` Dmitry Antipov
  2014-08-27 18:22         ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Antipov @ 2014-08-27 17:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On 08/27/2014 08:54 PM, Eli Zaretskii wrote:

> But your change still assigns values to variables, so I see no real
> difference.

? This change completely eliminates Vlast_code_conversion_error
and all assignments to it, plus related Qinsufficient_source,
Qinvalid_source and Qinterrupted.  Although we still need
coding->result, we don't need to expose relevant information
to Lisp.

Dmitry




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Unused/obsolete `last-code-conversion-error'?
  2014-08-27 17:21       ` Dmitry Antipov
@ 2014-08-27 18:22         ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2014-08-27 18:22 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: emacs-devel

> Date: Wed, 27 Aug 2014 21:21:15 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> On 08/27/2014 08:54 PM, Eli Zaretskii wrote:
> 
> > But your change still assigns values to variables, so I see no real
> > difference.
> 
> ? This change completely eliminates Vlast_code_conversion_error
> and all assignments to it

Exactly.  Instead, you assign values to internal struct members not
exposed to Lisp.  Sounds like a net loss to me.

> plus related Qinsufficient_source, Qinvalid_source and Qinterrupted.

So you saved 3 slots in obarray, why is that important?

> Although we still need coding->result, we don't need to expose
> relevant information to Lisp.

This is Emacs, not some OO language.  Maximum encapsulation is not a
merit around here.

IMO, this change is pointless, it removes features for no gain.  It
should not be done.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-08-27 18:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-27 13:09 Unused/obsolete `last-code-conversion-error'? Dmitry Antipov
2014-08-27 14:53 ` Eli Zaretskii
2014-08-27 16:21   ` Dmitry Antipov
2014-08-27 16:54     ` Eli Zaretskii
2014-08-27 17:21       ` Dmitry Antipov
2014-08-27 18:22         ` Eli Zaretskii

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.