This is an archive of the discontinued LLVM Phabricator instance.

[llvm-ml] Remove all file extension restrictions
ClosedPublic

Authored by ayzhao on Jun 2 2022, 5:05 PM.

Details

Summary

After D126425 was submitted, hans@ observed that MSVC's ml.exe doesn't
care about the file's extension at all. Now, we check if the file exists
to determine whether an input filename is a valid assembly file.

To keep things consistent with clang-cl and lld-link, llvm-ml will treat
everything that's not a flag as a filename.

Diff Detail

Event Timeline

ayzhao created this revision.Jun 2 2022, 5:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2022, 5:05 PM
ayzhao requested review of this revision.Jun 2 2022, 5:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2022, 5:05 PM
hans added a comment.Jun 3 2022, 4:42 AM

However, this is a problem on *nix systems as a leading forward slash
there is a valid file path. To partially mitigate this, we now consider
such inputs invalid arguments if they begin with a forward slash and do
*not* contain any more further forward slash characters. Everything else
is treated as a filename and we emit an appropriate error.

Both clang-cl and lld-link have the same problem. The way they handle it, they treat everything that's not a flag as a filename. For example:

clang-cl /c /nnlogo
clang-cl: error: no such file or directory: '/nnlogo'; did you mean '/nologo'?
clang-cl: error: no input files

lld-link /nnlogo a.obj
lld-link: error: could not open '/nnlogo': no such file or directory; did you mean '/nologo'

The "did you mean" is perhaps not strictly necessary, but makes it a little more user friendly.

Could we do the same for llvm-ml?

ayzhao updated this revision to Diff 434065.Jun 3 2022, 10:37 AM

Treat everything that's not a flag as a filename

ayzhao edited the summary of this revision. (Show Details)Jun 3 2022, 10:37 AM

However, this is a problem on *nix systems as a leading forward slash
there is a valid file path. To partially mitigate this, we now consider
such inputs invalid arguments if they begin with a forward slash and do
*not* contain any more further forward slash characters. Everything else
is treated as a filename and we emit an appropriate error.

Both clang-cl and lld-link have the same problem. The way they handle it, they treat everything that's not a flag as a filename. For example:

clang-cl /c /nnlogo
clang-cl: error: no such file or directory: '/nnlogo'; did you mean '/nologo'?
clang-cl: error: no input files

lld-link /nnlogo a.obj
lld-link: error: could not open '/nnlogo': no such file or directory; did you mean '/nologo'

The "did you mean" is perhaps not strictly necessary, but makes it a little more user friendly.

Could we do the same for llvm-ml?

Done

ayzhao updated this revision to Diff 434124.Jun 3 2022, 1:26 PM

Fix spacing

ayzhao updated this revision to Diff 434125.Jun 3 2022, 1:28 PM

more spacing fixes

LGTM, but I'll defer to hans here since he's guided thus far.

hans accepted this revision.Jun 7 2022, 1:56 AM

lgtm

This revision is now accepted and ready to land.Jun 7 2022, 1:56 AM
This revision was landed with ongoing or failed builds.Jun 7 2022, 10:03 AM
This revision was automatically updated to reflect the committed changes.
llvm/test/tools/llvm-ml/invalid_file_extension.blah