This is an archive of the discontinued LLVM Phabricator instance.

Set path syntax for remote executable FileSpec.
ClosedPublic

Authored by chaoren on May 7 2015, 2:40 PM.

Diff Detail

Event Timeline

chaoren updated this revision to Diff 25238.May 7 2015, 2:40 PM
chaoren retitled this revision from to Set path syntax for remote executable FileSpec..
chaoren updated this object.
chaoren edited the test plan for this revision. (Show Details)
chaoren added reviewers: zturner, ovyalov.
chaoren added a subscriber: Unknown Object (MLST).

Using denormalize = false while getting the executable path was a hacky workaround. This feels more correct (and is probably why m_syntax was added in the first place).

source/Host/common/FileSpec.cpp
819

I tried putting this (and the one at line 614) in an unnamed namespace, but the resulting executable couldn't even run. Bug in MSVC?

zturner added inline comments.May 7 2015, 2:50 PM
source/Host/common/FileSpec.cpp
824–825

Is it possible for the directory to end with a slash even when it has more than one character? For example, on Windows I can imagine the directory might be D:\.

So I think we should check m_directory.ends_with or some simialr mechanism of looking at the last character, and not doing a full comparison. Regardless of platform, you don't want to have a double slash, so I think the check is valid regardless.

chaoren added inline comments.May 7 2015, 4:05 PM
source/Host/common/FileSpec.cpp
824–825

FileSpec::SetFile actually strips D:\ into D:, but I think it's a good idea to do that regardless.

chaoren updated this revision to Diff 25256.May 7 2015, 4:05 PM

Address review comment. Remove duplicate code.

This revision was automatically updated to reflect the committed changes.