Getting the status of a sys::path curently requires new'ing the status object. It would be nice if we could make a subclass PathWithStatus, and move the 'status' related stuff there. This would give us something like this: class PathWithStatus : public Path { Status S; bool IsStatusValid; } If you sink the 'status gathering' methods out of Path into PathWithStatus, then you let the client decide whether they want to store a status or not, and if they want it, they don't have to pay for a separate malloc. -Chris
This isn't quite trivial. A lot of regular "Path" methods depend on the ability to get the file status. For example, the "isExecutable" method has to check both the access bits *and* determine if its a regular file. The method wants to only indicate true for executable files, not searchable directories. Consequently it needs the stat information. There are several other examples of these that I'm not quite sure how to resolve. From an API perspective it doesn't make sense to move these methods to PathWithStatus. From a performance perspective, it doesn't make sense to instantiate a PathWithStatus temporarily in the body of such methods. I'll work on this and come up with a solution.
The solution is to just use stat(2) directly. In all of these cases it would be bad if we were using stale stat information so the "force udpate" parameter was being passed to getFileStatus anyway which means that just using stat(2) directly preserves current behavior and simplifies things a lot.
Implemented with these patches: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070402/046965.html http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070402/046966.html http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070402/046967.html Jeff: I think I got the Win32 part of this right, but it hasn't been compiled. Could you please try it and verify on Win32?
Question: why not move isExecutable etc to PathWithStatus? -Chris
Because isReadable and isWritable need to be on Path and it didn't seem like a good API decision to move it to PathWithStatus. As I described, however, this needs re-stat on each call in case the status changed since the last time it was checked. So, the current approach (always stat) is both correct and equivalent to what we had previously when getFileStatus was part of Path class. Its also less impact to LLVM to leave it in Path class.