This is an archive of the discontinued LLVM Phabricator instance.

Fix temporary file name on Windows
ClosedPublic

Authored by yaxunl on Nov 13 2020, 6:47 AM.

Details

Summary

Bound arch may contain ':', which is invalid in Windows file names.

This patch fixes that.

Diff Detail

Event Timeline

yaxunl requested review of this revision.Nov 13 2020, 6:47 AM
yaxunl created this revision.
yaxunl updated this revision to Diff 305127.Nov 13 2020, 6:52 AM

add end of line to test

tra accepted this revision.Nov 13 2020, 10:33 AM

LGTM overall, nit about the character choice.

clang/lib/Driver/Driver.cpp
4645

Windows cmd shell uses % to reference environment variables. The character is valid for a path, but that would be similar to using $ in the path name on Linux -- doable, but rather inconvenient to deal with.

Perhaps a less special character would be a better choice. _, ., or @ ?

This revision is now accepted and ready to land.Nov 13 2020, 10:33 AM
yaxunl added inline comments.Nov 13 2020, 12:28 PM
clang/lib/Driver/Driver.cpp
4645

I would choose '@' if '%' is not favored. In some cases we may want to recover the bound arch from the temporary file name. '.' and '_' makes it difficult to recover.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2020, 5:11 AM