[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

Hmm. As you worried yourself in https://bugs.llvm.org/show_bug.cgi?id=27234#c3 we might be getting a worse user experience in some cases. For example:

  $ clang x.o
  clang: error: no such file or directory: 'x.o'
  clang: error: no input files
  
  $ clang -Wl,x.o
  /usr/bin/ld: cannot find x.o: No such file or directory
  clang: error: linker command failed with exit code 1 (use -v to see invocation)

We would now generate the latter message. Which doesn't really look much worse to me. I guess we might have wasted time by doing some compiles before invoking the linker, though..

GCC matches the current Clang behaviour though:

  $ gcc x.o
  gcc: error: x.o: No such file or directory
  gcc: fatal error: no input files

I wonder if there are deeper reasons for GCC and Clang's current behaviour than early diagnosis and slightly nicer error messages.

Since cl.exe seems to match the behaviour your patch is switching to, maybe we should only do this in clang-cl mode?



================
Comment at: clang/lib/Driver/Driver.cpp:2141
-      // and library files in paths we don't know about. Don't error in such
-      // cases.
-      return true;
----------------
Sounds hacky, who wrote that? Oh, I see :-)


================
Comment at: clang/lib/Driver/Driver.cpp:2165
+  // So do not diagnose this on the driver level. Rely on the linker diagnosing
+  // it. (If we don't end up invoking the driver, this means we'll emit a
+  // "'linker' input unused [-Wunused-command-line-argument]" warning instead
----------------
you mean "If we don't end up invoking the *linker*"?


================
Comment at: clang/lib/Driver/Driver.cpp:2173
+  // flag. (Users can use `-Wl,` or `/linker` to launder the flag past the
+  // driver in the unlikely case they run into this.)
+  if (Ty == types::TY_Object)
----------------
thakis wrote:
> thakis wrote:
> > This means that typo suggestion now works even if you pass `/link`. Before this patch, it didn't:
> > 
> > ```
> > % out/gn/bin/clang-cl /Brepo -### clang/test/Driver/unknown-arg.c /link
> > clang: error: no such file or directory: '/Brepo'; did you mean '/Brepro'?
> > ```
> > 
> > I suppose this means that before, we unintentionally and silently passed flags that clang-cl didn't know to link.exe even if they were in front of `/link`!  …and we _still_ do this for flags that aren't within editing distance of 1 of existing clang-cl flags:
> > 
> > ```
> > % out/gn/bin/clang-cl clang/test/Driver/unknown-arg.c /asdfasdf -fuse-ld=lld /link
> > lld-link: error: could not open '/asdfasdf': No such file or directory
> > ```
> > 
> > Now we even do this if there's no `/link` flag:
> > 
> > ```
> > % out/gn/bin/clang-cl clang/test/Driver/unknown-arg.c /libpath:foo -fuse-ld=lld
> > # fine
> > ```
> > 
> > That's kind of weird! With `-` we do something better:
> > 
> > ```
> > % out/gn/bin/clang-cl clang/test/Driver/unknown-arg.c -libpath:foo -fuse-ld=lld
> > clang: warning: unknown argument ignored in clang-cl: '-libpath:foo' [-Wunknown-argument]
> > ```
> > 
> > It's the old "things starting with '/' might be path, or might be flag" thing again, and as the --chroot example in this comment shows, it actually might be a path even if no file exists at that particular path. Hrm.
> Maybe we should tack on a `&& !(IsCLMode() && Value.startswith("/"))` here for that? (lld-)link.exe don't have a feature like `-chroot` as far as I know. It's a bit heuristic-y, but probably better than not doing it?
Hmm, yeah it sounds reasonable.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109624/new/

https://reviews.llvm.org/D109624