[PATCH] D15306: [llvm-profdata] Add support for weighted merge of profile data (2nd try)

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 7 16:10:54 PST 2015


Does it work with Window's native API: GetFileAttributes?

David

On Mon, Dec 7, 2015 at 4:05 PM, Nathan Slingerland <slingn at gmail.com> wrote:
> That's not to say that we shouldn't fix the issue in sys::fs::exists() if
> those functions are expected to transparently handle mixed path delimiters.
>
> On Mon, Dec 7, 2015 at 4:02 PM, Nathan Slingerland <slingn at gmail.com> wrote:
>>
>> This check also addresses the edge case of a path containing a ':' but the
>> file does not exist.
>> Without it the input would be interpreted as specifying a weight -
>> reporting an invalid weight error - rather than file does not exist.
>>
>> On Mon, Dec 7, 2015 at 3:45 PM, Justin Bogner <mail at justinbogner.com>
>> wrote:
>>>
>>> David Li via llvm-commits <llvm-commits at lists.llvm.org> writes:
>>> > davidxl added inline comments.
>>> >
>>> > ================
>>> > Comment at: tools/llvm-profdata/llvm-profdata.cpp:198
>>> > @@ +197,3 @@
>>> > +  if (WeightStr.find_first_of("/\\") != StringRef::npos) {
>>> > +    // Weight string contains a path delimiter - something went wrong
>>> > with
>>> > +    // parsing (likely due to platform-specific behavior with mixed
>>> > path
>>> > ----------------
>>> > Can you add a comment indicating this works around a bug in
>>> > sys::fs::exists()?
>>>
>>> Shouldn't we just fix sys::fs::exists()? This check seems questionable.
>>
>>
>


More information about the llvm-commits mailing list