Use TempFile in the implementation of LockFileManager

Authored by espindola on Nov 13 2017, 4:50 PM.



This move some of the complexity over to the lower level TempFile.

It also makes it a bit more explicit where errors are ignored since we now have a call to consumeError.

Diff Detail

rafael created this revision.Nov 13 2017, 4:50 PM
benlangmuir requested changes to this revision.Nov 14 2017, 11:18 AM
benlangmuir added a subscriber: bruno.

Spotted one issue, but otherwise seems okay to me. I'd like @bruno to take a look at this, because he's thought about this code much more recently than me.


The constructor is being called with a temporary lambda, so this is going to create a use-after-free.

This revision now requires changes to proceed.Nov 14 2017, 11:18 AM

Use std::function instead of function_ref.

Ping. This is now the last use of this sys::fs::createUniqueFile variant in llvm proper.

rnk accepted this revision.Nov 17 2017, 11:40 AM

lgtm, let's give it a go.


This is total bikeshedding, but I like to write these RAII helpers inline, like:

struct TempFileRemover {
  TempFile &TF;
  LockFileManager &LFM;
  bool Cancelled = false;
  TempFileRemover(TempFile &TF, LockFileManager &LFM) : TF(TF), LFM(LFM) {}
  ~TempFileRemover() {
    if (Cancelled)
    if (Error E = TF.discard())
} RemoveTempFile(*TempFile, *this);

It saves a std::function, but you have to write out the capture members and a constructor to initialize them. Maybe this is just what happens when your brain has been damaged from working on a C++ compiler too long.


I kept the RAII class outside the function since a cancelable cleanup seems a general concept and we might want to move it to a header in the future.

bruno accepted this revision.Nov 17 2017, 6:33 PM

LGTM as well!

espindola commandeered this revision.Mar 6 2018, 5:19 PM
espindola abandoned this revision.
espindola added a reviewer: rafael.