Back to the schedule
Previous: telega.el and the Emacs community on Telegram
Next: Emacs Research Group, Season Zero: What we did together with Emacs in 2 hours a week for a year

A day in the life of a janitor

Stefan Monnier

Q&A: live Q&A or IRC
Status: Finished
Duration: 25:57

This talk will also be streamed at an alternate time for APAC hours: https://libreau.org/upcoming.html#emacsconf21

If you have questions and the speaker has not indicated public contact information on this page, please feel free to e-mail us at emacsconf-submit@gnu.org and we'll forward your question to the speaker.

Description

Because of a reckless former Emacs maintainer that shall better stay unnamed, ELisp has seen a fair bit of churn in the last 10 years, making it necessary to clean up "old" code [in order to open up the road for yet more recklessness? ]. In this documentary we will follow a famous janitor in his every day job dealing with the aftermath of the cl-lib / lexical-binding party.

Discussion

  • Question: Is there a place where these conventions and compilers checks are listed? A web page, an info perhaps?
  • Wow, I think you are going to have to know a LOT on Emacs development, versions, Elisp details, ... to do that kind of work.
    • One very helpful thing anyone can do is just confirm bug reports and add reproduction steps if they are missing.
  • The double-dash is a convention for an func intended to be called internally only?
    • Yes. pacakge-foo is public, package--foo is internal.
  • mindless tree-wide transforms like this are what coccinelle is for. why are emacs maintainers still doing this by hand? you'd think lisp would be well-suited for expressing semantic patches to lisp... :/ this ceases to seem interesting when you've seen cocci do ten thousand transforms like this across a 500MiB tree in 20s or so
    • Does cocci work on elisp?
      • no, but the idea should work there, and Lisp is so regular in structure that something like coccinelle is the sort of thing lisp boosters say is really easy in lisp. but nooo, none exists that I know of :( (coccinelle itself is written in ocaml :) )
      • https://coccinelle.gitlabpages.inria.fr/website/
  • There's a monstrous heap of regexps that match most reasonable compiler output. I wrapped an ancient MS-DOS compiler for an obsolete language in a script that invokes it inside DOSBox and echos the output‚ and it Just Worked with compilation mode.
    • Wow, that's awesome! Yes, lots of the "historic" parts of emacs are amazing.
  • if folks are interested in the lexical dynamic transition: https://hopl4.sigplan.org/details/hopl-4-papers/13/Evolution-of-Emacs-Lisp

Feedback:

  • OK, this blows my mind in a sense that I realize that I really don't have an idea of coding Elisp.
  • This talk is great. There should be more like that online, so more people learn to help with "janitorial" work

Outline

  • ~20 minutes Here really, I'm not sure how much time this will take. I put 20 minutes because I think I might be able to fill that and I think more than that could turn too boring. I intend to make it a "live coding" kind of thing, without anything like an outline: it's basically "make" followed by fixing the warnings.

Transcript

Hello, my name is Stefan Monnier, and I'm going to talk to you about-- well, I'm going to present a bit of the life of a janitor.

[00:00:11.840] So by and large, there's just nothing to see here, and that's probably not super interesting, but some of you might actually like to see how I work, so I figured why not.

[00:00:25.359] Usually what I do just doesn't make any any significant difference, and so I basically take existing code that's working, and I try to change it hopefully without breaking it too much and make it slightly more... you know, following some of the more modern style, let's say, and sometimes along the way, it actually fixes some bugs.

[00:00:50.399] More concretely, the kind of things that I do is basically activate lexical scoping-- that's really my main goal usually-- but also do things like convert from cl to cl-lib, sometimes I have to fix some compilation dependencies, I might convert from defadvice to advice-add, and many of the things-- in terms of number of changes, most of them are actually changing quote fun to hash quote fun because I prefer it, but also it often helps me have a better understanding of which function is called where, and so the warnings I get from it sometimes help me. You look concretely... it's not nothing really clear; it's more in terms of helping me have a mental image of how the package works.

[00:01:39.439] So let's take a look. I'm going to start with the package heap, which I saw had a few weird things in it, so I'm going to compile it. That's basically the way the way I work, right. I take a package. I just pass it to the byte compiler. I do that by just having a clone of the whole GNU ELPA repository, and so that's why I built them. I use the build rules from the GNU ELPA repository.

[00:02:15.120] These build rules enforce-- make sure that the files are compiled in a clean environment so you get fairly good warnings. If you look at the warnings you see here, there's a lot of things which are completely irrelevant, which are due to details of the way I have my Emacs set up and some of the local changes I had in it so, you know, there's no point paying too much attention to it, but here we have a first warning. We see that this is using cl, so we want to change this to cl-lib, but that also means that we may have a new dependency on the cl-lib package, so we have to go check the start of the file to see if it already declares some dependency, and we see it doesn't, not even on a on a recent-enough Emacs, so we have to add-- sorry, that's not going very well... oh... okay, we're going to get there somewhere... somehow... oh, that still wasn't it, wow, okay-- and along the way...

[00:03:22.159] Of course, since we converted to cl-lib, we have to update the uses so defstruct shouldn't be used anymore. We may want to reindent this to get something a bit cleaner. We have here a missing quote... hash, sorry. We have decf, so decf is here, and that needs to be replaced with cl-decf. Sometimes it's worth doing a search-and-replace. Here I see there's only two, so it's not worth the trouble; I just do it by hand, and that's it. Well, that was easy. So let's recompile, see what it says. Ah, this is clean. Perfect!

[00:04:12.959] Let's.. we can go see... There is another one I had. Was it counsel, I think. Yes. So also I saw some funny things going on here. So I'm going to do the same procedure as before: I just compile the file and look at the warnings. Oh, we have many more here. So let's see... Okay, so we have missing quotes-- oh, hashes. They're not really missing; it's just a personal preference.

[00:04:54.639] Oh, here... here's an important one: so as you know, if you look at the top of the file, you see that here it says it's using lexical binding, yet it's not fully using lexical binding, because as we just saw, there's a call to the eval function with only one argument, which means the second argument is nil, which means that the expression read by read here is going to be evaluated using the old dialects, which is only dynamic scoping. So here I like to just change this to use lexical scoping, which in most cases just doesn't make any difference. It just makes me feel better.

[00:05:35.919] So there's lots of those hashes all over the place. It's not strictly necessary, as you know, but I'm just going to add them anyway.

[00:05:52.479] Here we see it's not going to warn me here because it doesn't know that ivy-make-magic-action takes a function, but it's a pretty good guess that it does. And here's some more. What else do we have? Is that all we have here? Well, looks like it. Oh, I see a few... a few more here... and one more.

[00:06:30.639] And oh, this is more interesting. So here we have a use of defadvice, so if we go back to the beginning of the file, we see that it actually depends on Emacs 24.5, so it actually has the new advice system available without having to add any dependency, so there's really no good reason to keep this. So we just convert this to an advice-add, so it just says, you know, this is the function that's advised. This was a before advice. The before advice, sometimes, when we convert it to advice-add, need to be converted to around advice. This is when the function looks or modifies the argument. In this case, if I look at it, I see it doesn't seem to be using the arguments at all. So I'm just going to keep it as a before advice. And we have to give it a name. Well, we don't really have to, but it's convenient to give it a name to the new function. So here, they actually had given a name to the advice, so we're going to keep it, and indeed it's the only function. This name is not used as a function, so we can use it as the name of the function. I'm going to add a dash here because I think this function is really fundamentally an internal function. So here I just said I add the advice, but I still need to actually define the function. So that's what I do here, and we need here to list the arguments that are going to be taken. I don't know what these are, but I know we're not using them, so we'll just accept anything, and that will do the trick. It's a future-proof as well, so that should work.

[00:08:22.240] Oh, here we have another, so it's basically the same story, I think. It's a before advice as well. It doesn't seem to be using the argument at all, and let's see if this name is not taken. Yeah, good, so we can just do the same: turn this into an advice-add... before... I just add a dash here. And same thing-- a function that just takes... because I don't know which arguments these are so... I think that should do the trick. Actually, we see that this function is very similar to the other one. Let's look at the two side-by-side... ...it really is-- oh, it's not exactly identical... it's, you know, we could try to merge them into a single function, but it's probably not worth the trouble so we can keep it this way.

[00:09:45.920] Okay, next warning: an eval again, so I could just add t here, but if you look at it a bit more, you see that the code we're going to evaluate using either lexical scoping or dynamic scoping is actually just evaluating a symbol, since we just call an intern here. So instead of replacing this by adding an argument, I'm just going to call symbol-value because that's exactly what we need to do here, right. I call this "strength reduction," and I'm using a more primitive function instead, which does just what we need, and this one knows that it has to be accessed by dynamic scoping, of course.

[00:10:30.640] Here I have a kmacro-ring, so here I have a function that uses-- kmacro-ring comes from the kmacro package, obviously, and we probably don't want to require kmacro package all over the place in counsel itself, because counsel can be used without kmacro. So I think we're just going to add a defvar to silence the warning. And we have several more. So we have initial-counter-value. (Sorry.) We have kmacro-counter. Do we have more? Oh, yes, we do. We have kmacro-counter-value-start and kmacro-counter-format-start. Okay. I hope this is it. kmacro-ring, counter, ring... blah blah blah. Here we have another one, quote. Here we have another hash missing. It's not missing... but same thing here. Okay, this is a function from kmacro. We could declare it just to silence the warning although we don't actually... normally, when we declare such things-- same thing with variables-- we should try to make sure that indeed by the time the code is executed, the function will be available, and then very often is because there's a require sometimes inside a function, and so we should put the declare function right after the require, but I don't think it's the case here. So I'm just going to to add this. I know this comes from kmacro, and I could actually check the arguments. It's just taking an optional argument so I'm going to put it there, so we have it complete.

[00:13:06.720] Okay, we can just recompile, see what is left from those warnings we've fixed, and we may have new warnings, in any case, because especially when we add the hashes, it tends to give us more warnings. So we have two more functions which are not known. You can just add them here... set-format "kmacro" and same thing for set-counter. Okay, whatever. This just takes a format argument, and this one just takes an arg argument. Okay, so let's see what this says now. Hopefully, there's no warnings anymore. We're done. Okay!

[00:14:17.839] Okay, the last one we're going to see is in enwc, I saw the other day... I think I have it here... here we go, yes... so enwc is an interesting package here because it has-- as you can see it has-- it's lexical binding, but actually some of the files in it do not use lexical binding, so it has been partly converted but not completely. So here I'm going to enable lexical binding. I have also, I think, in cm... yes... so I enable it here, and also, I think, test.

[00:15:07.360] The test files are often somewhat problematic because very often they're not quite as heavily tested themselves, actually, or they only run in very specific contexts, and so they may have problems with missing requires or using packages which are not explicitly in the dependencies and those kinds of things. I think this is not the case here, but we'll see. enwc... Yes, I want to save this one and that one.

[00:15:42.320] Let's see what it says. Okay, unused lexical variable x... x... Yes, so here we have an unused variable, and indeed, it's not used. It probably had to be named before because it was... with dynamic scoping, the dotimes requires the variable to be named, actually, because it's used internally somehow, but with lexical scoping, that's not the case, so we can just put an underscore.

[00:16:14.079] I'm going to change this because I really don't like this three-part dotimes. I prefer to have the return value at the end. It's sort of stashed hidden in the middle. That's just a personal preference.

[00:16:29.680] Okay, what else... we have a widget. Okay, this argument here says that it's not used, so if we look at... We were here, right? Yes. Right here. Indeed, widget is really not used. (Sorry.) Here's what I get for using a somewhat vanilla configuration of Emacs, compared to the one I use... the personally tricked one. Actually, I can... so we can just mark this argument as unused, and we don't want to remove the argument probably, or maybe we could; we could see where the function is used, and here we see that it's passed to a higher-order function, basically, so it's going to be... We can't really change the calling convention so we have to mark the argument as being just an unused argument, but we're going to still receive it. And here it says same thing: that widget is not used in this function. Let's take a look at the function. Indeed it seems it's not used, and so we're just going to mark it as unused. This is the part of the conversion to lexical scoping that's somewhat tricky sometimes because we don't really know whether this variable should be using lexical scoping or dynamic scoping. The fact that it's not used is a hint that there's probably something going on, so either it's not used because it should be using dynamic scoping-- it is going to be used by some other code somewhere else-- or it's really not used because it's just not used, right, and so we need to distinguish the two, and for that, I basically use my own judgment. This is based typically on the fact that this is just a very short name, and most local identifiers use short names, whereas item values used for dynamic scoping typically have a package prefix or something like this. So the fact that it's a short name gives me a good idea. Here in this case, I actually look at the code, and we see that there's nothing in here that may actually refer to this variable widget, so I think it's safe, but in the general case, we may look here and be surprised, or, you know, you may call out the functions which may themselves end up referring to this variable. So sometimes we need to investigate a little more. We are most of the time not completely sure whether the result is correct or not, of course, so the other thing you may want to check is also uses of things like eval or symbol-value. So it's often a good idea to search, and you do a search of eval, and you see here it's using eval. Hmmm... Okay, so what does this eval do? It's evaluating expressions that appear in args here so you can see where those args come from, and we see here, these are expressions that don't do anything very special. It's just using make-supplicant-choice, and make-supplicant-choice itself just doesn't refer to widget, for example, so you know we should be safe, but while I'm here... okay, well, then we can do that later.

[00:19:53.840] Well, that's actually the next warning, exactly. So here we see that this is using the dynamically-scoped dialect, so we convert it to lexical-scoped. Of course, this may introduce errors, but we hope it doesn't. And actually, it was a good change here, because if you see again, this actually evals expressions that appear here in args, and so these are expressions that are passed here. So this expression here used to be evaluated with dynamic scoping, even though it appears to be normal code within this file, which says it's using lexical scoping, and so there are some remnants of dynamic scoping all over the place in Emacs still, because we have those calls of eval with a nil argument. Here we have cons... that needs to be hash quoted.

[00:20:52.400] Oh, and we have a reference to this variable `enwc-edit-id'. So this is clearly a dynamic-scoped variable. We can either add a defvar to silence the warning, or maybe we can require the package. The file that defines it... So let's see where it's defined. Here it's defined in enwc.el, so I'm going to try just to add the dependency. I'm going to require here. This is risky. We'll see when we compile a file later, we may get a circular dependency because of it. If that's the case, we're going to have to remove this require and instead put defvars. Sometimes it's worth actually looking further at the various files to see how to redefine the dependencies to break those circular dependencies, but it's often not really worth the trouble. Oh, no, that's not what-- I'm not going to the right place... Here I was. So here edit-map. Well, we can probably... it may disappear or... oh, I see. Okay, so this edit-map actually is defined in this very file. It's just that it's defined later. So all we need to do is to move this definition to before its first use, since otherwise it's going to be taken as lexically-scoped, which we don't want.

[00:22:33.520] And while I'm here, I see this copy-keymap. I don't like copy-keymap, so I'm going to change this to a normal keymap, and then I'm just going to use set-keymap-parent instead of copy-keymap to get basically the same result, but without having copied anything. And this one will disappear... this one as well-- or should hopefully, thanks to the require. Here we have a hash missing, and we have some functions which are unknown, so let's see... Where is this function defined? Nowhere. Huh, wonderful, okay. So we'll just leave it like it is, and that's going to be for the author of the package to fix. How about this one? Oh, okay, so it's defined in enwc.el so presumably, this is going to disappear as well. One more... Okay, so this one is just like the previous one. We're going to leave it at that. And this is it! Huh, wonderful. So let's recompile. Oh, we have a warning for fin. This variable seems not to be used anywhere in the file, so we're just going to remove it. I leave it there just in case someone needs later on to look for a fin variable to see where it used to be. Again, you know, maybe it's actually used... yeah, dynamic scoping somehow, but given the short name, I presume this is not the case. Here, oh, that's the code removed that had a hash missing. That's the one that's not defined. This one is not defined, and this is it.

[00:24:58.000] Let's make a last recompilation to see if we missed yet something else. Nope, and that's it, okay. Well, here we go; we're done. Okay so this was it. You've seen, I think, pretty much examples of all of those, and I hope you enjoyed it. Lessons to take home: use the byte compiler. You can also use flymake-mode instead. I recommend enabling it as much as you can, and head the warnings. Follow the warnings. Try to fix them. If you can fix all of the warnings, it's always much better, because then the new warnings really show up. And once you've done it, it's really kind of-- because there's always new things coming up. And I think this is it. I hope you liked it, and thank you for attending this presentation. Bye. (captions by Hannah Miller)

Back to the schedule
Previous: telega.el and the Emacs community on Telegram
Next: Emacs Research Group, Season Zero: What we did together with Emacs in 2 hours a week for a year