Page MenuHomePhabricator

[Support] Add path::temp_directory() function.
AbandonedPublic

Authored by chfast on Nov 24 2015, 9:39 AM.

Details

Summary

Function path::temp_directory() returns a path to OS-specific temporary directory. On Windows and OSX it is usually a dir under user's control, on Linux it is usually /tmp.
This function is going to replace path::system_temp_directory(true).

Diff Detail

Event Timeline

chfast updated this revision to Diff 41059.Nov 24 2015, 9:39 AM
chfast retitled this revision from to [Support] Add path::temp_directory() function..
chfast updated this object.
chfast added reviewers: rafael, aaron.ballman.
chfast added a subscriber: llvm-commits.
aaron.ballman added inline comments.Nov 24 2015, 12:26 PM
include/llvm/Support/Path.h
336

Why is Path1 required to pass to the function?

lib/Support/Unix/Path.inc
563

Is there a reason this is a bool instead of an int?

646

Do you have to call Result.clear() here in the event getDarwinConfDir() fails but modifies Result?

lib/Support/Windows/Path.inc
816

I know this isn't yours, but this is a terrible idea just the same. We should not be hard coding paths on Windows. I have a slight preference for this API to return a bool indicating failure, but at the same time, I struggle to imagine what error recovery would look like. I think I could be persuaded that failures here should assert and fail.

824

Do you need to clear Result if user_cache_directory() fails?

chfast added inline comments.Nov 25 2015, 5:55 AM
include/llvm/Support/Path.h
336

It was discussed around user_cache_directory(). It is not good idea to place files there directly so the API enforces appending at least one path element.

http://reviews.llvm.org/D13801?id=37977#inline-115640

lib/Support/Unix/Path.inc
563

Not any particular reason. I was replacing bool flag, so I used bool.

646

getDarwinConfDir() either clears or does not modify Result in case of failure.

lib/Support/Windows/Path.inc
824

No. user_cache_directory() will either clear or not modify Result in case of failure.

aaron.ballman added inline comments.Nov 25 2015, 6:45 AM
include/llvm/Support/Path.h
336

That's not the case for the temp directory on Windows, but is the case for the user_cached_directory(). Temp files generally live directly in the temp directory on Windows, and folders are only used when there is a chance the file names will conflict with an existing file. However, the usual pattern when in the temp directory is to pick a temp file name, and so those conflicts do not happen. However, with the user's cache directory, those files are longer-lived, and so they tend to have specific file names that can conflict.

lib/Support/Unix/Path.inc
563

I'm not keen on the bool because then comparisons below are effectively if (bool == bool) instead of if (bool). I have a slight preference for unsigned here.

chfast abandoned this revision.Sep 13 2016, 1:02 AM