[llvm-ml] Remove all file extension restrictions

[ayzhao]

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.

[hans]

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]

Treat everything that's not a flag as a filename

[ayzhao]

In D126931#3555677, @hans wrote:

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]

Fix spacing

[ayzhao]

more spacing fixes

[epastor]

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

[hans]

lgtm

llvm/test/tools/llvm-ml/basic.test

 # RUN: not llvm-ml %t.blah.asm /nologo /Fo /dev/null 2>&1 | FileCheck -DMSG=%errc_ENOENT --check-prefix=ENOENT %s
+# RUN: not llvm-ml /Ta /nnlogo /Fo /dev/null 2>&1 | FileCheck -DMSG=%errc_ENOENT --check-prefix=ENOENT-TA %s
+# RUN: not llvm-ml - /Fo /dev/null /nnlogo 2>&1 | FileCheck -DMSG=%errc_ENOENT --check-prefix=ENOENT-FLAG %s
 
 # ENOENT-NOT: nologo
 # ENOENT: {{.*}}.blah.asm: [[MSG]]
+# ENOENT-TA: /nnlogo: [[MSG]]
+# ENOENT-FLAG: error: /nnlogo: [[MSG]], did you mean '/nologo'?

llvm/test/tools/llvm-ml/invalid_file_extension.blah

-; RUN: not llvm-ml %s /Fo /dev/null 2>&1 | FileCheck %s --check-prefixes=CHECK-INVALID
-; RUN: llvm-ml /Ta %s -m64 -filetype=s /Fo - | FileCheck %s --check-prefixes=CHECK-TA
-
-; CHECK-INVALID: error: invalid option '{{.*}}invalid_file_extension.blah'
-
-.code
-foo:
-; CHECK-TA: foo:

llvm/test/tools/llvm-ml/lit.local.cfg

 if not ('X86' in config.root.targets):
     # We need support for X86.
     config.unsupported = True
 
 config.suffixes.add('.asm')
-config.suffixes.add('.S')
 config.suffixes.add('.blah')

llvm/test/tools/llvm-ml/random_file_extension.blah

+; RUN: llvm-ml %s -m64 -filetype=s /Fo - | FileCheck %s
+; RUN: llvm-ml /Ta %s -m64 -filetype=s /Fo - | FileCheck %s
+
+.code
+foo:
+; CHECK: foo:

llvm/test/tools/llvm-ml/valid_file_extension.S

-; RUN: llvm-ml -m64 -filetype=s %s /Fo - | FileCheck %s
-
-.code
-foo:
-; CHECK: foo:

llvm/tools/llvm-ml/Opts.td

 class UnsupportedJoinedOrSeparate<string name> :
         JoinedOrSeparate<["/", "-"], name>, Group<unsupported_Group>;
 class UnsupportedSeparate<string name> : Separate<["/", "-"], name>,
                                          Group<unsupported_Group>;
 
 def bitness : LLVMJoined<"m">, Values<"32,64">,
               HelpText<"Target platform (x86 or x86-64)">;
 def as_lex : LLVMFlag<"as-lex">,
-             HelpText<"Lex tokens from an .asm or .S file without assembling">;
+             HelpText<"Lex tokens from a file without assembling">;
 def debug : LLVMFlag<"debug">, Flags<[HelpHidden]>,
             HelpText<"Enable debug output">;
 def debug_only : LLVMCommaJoined<"debug-only=">, Flags<[HelpHidden]>,
                  HelpText<"Enable a specific type of debug output (comma "
                           "separated list of types)">;
 def fatal_warnings : LLVMFlag<"fatal-warnings">,
                      HelpText<"Treat warnings as errors">;
 def filetype : LLVMJoined<"filetype=">, Values<"obj,s,null">,

 def include_path : MLJoinedOrSeparate<"I">,
                    HelpText<"Sets path for include files">;
 def safeseh : MLFlag<"safeseh">,
               HelpText<"Mark resulting object files as either containing no "
                        "exception handlers or containing exception handlers "
                        "that are all declared with .SAFESEH. Only available in "
                        "32-bit.">;
 def assembly_file : MLJoinedOrSeparate<"Ta">,
-                    HelpText<"Assemble source file with name not ending with "
-                             "the .asm or the .S extension">;
+                    HelpText<"Assemble source file with the given name. Used "
+                             "if the filename begins with a forward slash.">;
 def error_on_warning : MLFlag<"WX">, Alias<fatal_warnings>;
 def parse_only : MLFlag<"Zs">, HelpText<"Run a syntax-check only">,
                  Alias<filetype>, AliasArgs<["null"]>;
 def ignore_include_envvar : MLFlag<"X">,
                             HelpText<"Ignore the INCLUDE environment variable">;
 
 def tiny_model_support : UnsupportedFlag<"AT">, HelpText<"">;
 def alternate_linker : UnsupportedJoined<"Bl">, HelpText<"">;

llvm/tools/llvm-ml/llvm-ml.cpp

int main(int Argc, char **Argv) {
   unsigned MissingArgIndex, MissingArgCount;
   ArrayRef<const char *> ArgsArr = makeArrayRef(Argv + 1, Argc - 1);
   opt::InputArgList InputArgs =
       T.ParseArgs(ArgsArr, MissingArgIndex, MissingArgCount);
 
   std::string InputFilename;
   for (auto *Arg : InputArgs.filtered(OPT_INPUT)) {
     std::string ArgString = Arg->getAsString(InputArgs);
-    StringRef ArgStringRef(ArgString);
-    if (ArgString == "-" || ArgStringRef.endswith(".asm") ||
-        ArgStringRef.endswith(".S")) {
+    bool IsFile = false;
+    std::error_code IsFileEC =
+        llvm::sys::fs::is_regular_file(ArgString, IsFile);
+    if (ArgString == "-" || IsFile) {
       if (!InputFilename.empty()) {
         WithColor::warning(errs(), ProgName)
             << "does not support multiple assembly files in one command; "
             << "ignoring '" << InputFilename << "'\n";
       }
       InputFilename = ArgString;
     } else {
       std::string Diag;
       raw_string_ostream OS(Diag);
-      OS << "invalid option '" << ArgString << "'";
+      OS << ArgString << ": " << IsFileEC.message();
 
       std::string Nearest;
       if (T.findNearest(ArgString, Nearest) < 2)
         OS << ", did you mean '" << Nearest << "'?";
 
       WithColor::error(errs(), ProgName) << OS.str() << '\n';
       exit(1);
     }