Less Than Exceptional

26/02/2017

As of a few weeks ago, I’ve become part of Evil’s new maintainer
team. Most of the bugs we get are not quite Evil’s fault, but
rather that of other components it relies upon, such as Emacs. #785
is a perfect example of this observation. Take a look at the
sources of the function responsible for the reported behavior and
see whether you can find what’s wrong about it:

DEFUN("lookup-key",Flookup_key,Slookup_key,2,3,0,doc:...)(Lisp_Objectkeymap,Lisp_Objectkey,Lisp_Objectaccept_default){ptrdiff_tidx;Lisp_Objectcmd;Lisp_Objectc;ptrdiff_tlength;boolt_ok=!NILP(accept_default);keymap=get_keymap(keymap,1,1);length=CHECK_VECTOR_OR_STRING(key);if(length==0)returnkeymap;idx=0;while(1){c=Faref(key,make_number(idx++));if(CONSP(c)&&lucid_event_type_list_p(c))c=Fevent_convert_list(c);/* Turn the 8th bit of string chars into a meta modifier. */if(STRINGP(key)&&XINT(c)&0x80&&!STRING_MULTIBYTE(key))XSETINT(c,(XINT(c)|meta_modifier)&~0x80);/* Allow string since binding for `menu-bar-select-buffer'
includes the buffer name in the key sequence. */if(!INTEGERP(c)&&!SYMBOLP(c)&&!CONSP(c)&&!STRINGP(c))message_with_string("Key sequence contains invalid event %s",c,1);cmd=access_keymap(keymap,c,t_ok,0,1);if(idx==length)returncmd;keymap=get_keymap(cmd,0,1);if(!CONSP(keymap))returnmake_number(idx);maybe_quit();}}

If you haven’t spotted it, the function never signals an exception for
the exceptional case of an invalid key sequence. Instead, it returns
one of the few types that aren’t allowed in a definition, namely an
integer. It’s up to the caller to handle this case on their own,
something the majority of Emacs packages rightfully neglect to do as
even + informs you about invalid input. This design decision
reeks of the many ways of signalling errors in C and worse, cannot
be properly fixed as there’s code inside Emacs relying on the integer
return type. There is even a lookup-key-ignore-too-long in Emacs
core that looks a lot like what I wrote, but it’s part of
menu-bar.el for whatever reason and not advertised at all.