uint32_t options -> File::OpenOptions options

[lawrence_danna]

Summary

This patch re-types everywhere that passes a File::OpenOptions
as a uint32_t so it actually uses File::OpenOptions.

It also converts some OpenOptions related functions that fail
by returning 0 or NULL into llvm::Expected

split off from https://reviews.llvm.org/D68737

[labath]

I originally thought you would do something local, but changing this across the board is definitely better, and it doesn't seem the patch is that big. LLVM already has a utility to work out the kinks in the bitmask enum use case. Please take a look at llvm/ADT/BitmaskEnum.h on how to make use of that.

lldb/include/lldb/Host/File.h

59

A better way to handle that is to make this a typed enum (enum OpenOptions : uint16_t). Then you can add something like LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/0xffffu) to make sure the bitmask operators also work well.

lldb/source/Host/common/File.cpp

42

I'd consider leaving the static function as const char * -- the function doesn't do anything complex, and there's only one way it can fail (which can be signalled by a nullptr), so having the Expected wrapper around that does not seem all that useful -- I'll leave it up to you though.

74

For similar reasons, I'd make this an Optional<OpenOptions>.

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

1093–1098

If GetOptionsFromMode stays as Expected then this is not correct, as it does not handle the error case. Overall, I'd say we shouldn't use auto here..

[lawrence_danna]

review fixes

[lawrence_danna]

lldb/source/Host/common/File.cpp

42

If I do that I'd still just have to convert it to Error in the callers, so I think I'll leave it this way.

[lawrence_danna]

fix

[labath]

lldb/source/Host/common/File.cpp

42

Yes, but OTOH, if you did that, there would also be callers where you wouldn't need the consumeError(.takeError()) stuff. Anyway, I think this is fine too...