[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-07-08 Thread Steven Wan via Phabricator via cfe-commits 
stevewan added a comment.

In D79842#2138857 , @DavidSpickett 
wrote:

> Right, I see the issue.
>
> The code that gets the default triple name 
> (https://reviews.llvm.org/D13340?id=36227#inline-108606) looks up the one you 
> have in cmake, not the actual default which you get in --version. We could 
> "fix" this by doing so when we make the tool name as well, but this breaks 
> whatever mips toolchain was using that. (their tools won't be 
> mips-unknown-elf-)
>
> So yes it looks up powerpc64le-linux-gnu but shows 
> powerpc64le-unknown-linux-gnu. Can't go back to using cmake's value because 
> on Mac OS, cmake has x86_64-darwin, clang has x86_64-darwin. Writing 
> to both is a short term option so I will try that and fold it into 
> https://reviews.llvm.org/D83055. (will add you on review once I update it)
>
> (this whole default triple lookup should probably go but I'd really like to 
> do that in its own commit)


Thanks. @DavidSpickett


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-07-08 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

Right, I see the issue.

The code that gets the default triple name 
(https://reviews.llvm.org/D13340?id=36227#inline-108606) looks up the one you 
have in cmake, not the actual default which you get in --version. We could 
"fix" this by doing so when we make the tool name as well, but this breaks 
whatever mips toolchain was using that. (their tools won't be 
mips-unknown-elf-)

So yes it looks up powerpc64le-linux-gnu but shows 
powerpc64le-unknown-linux-gnu. Can't go back to using cmake's value because on 
Mac OS, cmake has x86_64-darwin, clang has x86_64-darwin. Writing to 
both is a short term option so I will try that and fold it into 
https://reviews.llvm.org/D83055. (will add you on review once I update it)

(this whole default triple lookup should probably go but I'd really like to do 
that in its own commit)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-07-07 Thread Steven Wan via Phabricator via cfe-commits 
stevewan added a comment.

Yes, this issue was hit with the reland applied. When given a 
`--` format LLVM_DEFAULT_TARGET_TRIPLE, Clang would expand the 
target triple to `---`, and therefore causes the name 
mismatch between what the driver searches for and what the test case creates as 
the dummy tool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-07-07 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

I saw similar behaviour on Mac OSX and fixed that in the reland. 
(https://reviews.llvm.org/rGd6efc9811646edbfe13f06c2676fb469f1c155b1)

Are you still seeing a failure with that applied?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-07-06 Thread Steven Wan via Phabricator via cfe-commits 
stevewan added a comment.

The test was failing on Linux if I set LLVM_DEFAULT_TARGET_TRIPLE. For example 
if I set it to`powerpc64le-linux-gnu` clang actually uses 
"powerpc64le-unknown-linux-gnu".

Would you be able to provide a fix to this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-06-25 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett closed this revision.
DavidSpickett added a comment.

Relanded as d6efc9811646edbfe13f06c2676fb469f1c155b1 
. Change 
was to get the default triple from clang itself, as on MacOS this can be 
different to the value you set via CMake.

Closing this review as the diff shown was committed as 
028571d60843cb87e2637ef69ee09090d4526c62 
 but 
wasn't closed automatically because I had the wrong diff link in the commit msg.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-06-22 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

Committed but made a mistake with arc so it landed with the wrong diff number. 
In any case a failure on MacOS was reported so reverted while I investigate 
that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-06-17 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett updated this revision to Diff 271345.
DavidSpickett added a comment.

Moved pipe (|) to end of the first lines to
make it clearer that there's a continuation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c
  clang/test/lit.cfg.py

Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -46,6 +46,8 @@
 config.substitutions.append(
 ('%src_include_dir', config.clang_src_dir + '/include'))
 
+config.substitutions.append(
+('%target_triple', config.target_triple))
 
 # Propagate path to symbolizer for ASan/MSan.
 llvm_config.with_system_environment(
Index: clang/test/Driver/program-path-priority.c
===
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,106 @@
+/// Don't create symlinks on Windows
+// UNSUPPORTED: system-windows
+
+/// Check the priority used when searching for tools
+/// Names and locations are usually in this order:
+/// -tool, tool, -tool
+/// program path, PATH
+/// (from highest to lowest priority)
+/// A higher priority name found in a lower priority
+/// location will win over a lower priority name in a
+/// higher priority location.
+/// Prefix dirs (added with -B) override the location,
+/// so only name priority is accounted for, unless we fail to find
+/// anything at all in the prefix.
+
+/// Copy clang to a new dir which will be its
+/// "program path" for these tests
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: ln -s %clang %t/clang
+
+/// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: /gcc
+
+/// -gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+/// -gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+
+/// -gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+
+/// On program path, -gcc is preferred to plain gcc
+// RUN: touch %t/gcc && chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: /gcc
+
+/// -gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+
+/// -gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc && chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+
+/// -gcc has lowest priority so -gcc
+/// on PATH beats default triple in program path
+// RUN: touch %t/%target_triple-gcc && chmod +x %t/%target_triple-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
+// DEFAULT_TRIPLE_GCC: env/notreal-none-elf-gcc
+
+/// plain gcc on PATH beats default triple in program path
+// RUN: rm %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
+// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+
+/// default triple only chosen when no others are present
+// RUN: rm %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
+// DEFAULT_TRIPLE_NO_OTHERS: -gcc
+//

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-06-17 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett marked 2 inline comments as done.
DavidSpickett added inline comments.



Comment at: clang/test/Driver/program-path-priority.c:22
+/// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO_NOTREAL_GCC %s

MaskRay wrote:
> Nit: this may be the preference of some reviewers, but they tend to favor `| 
> \` in the end of the last line. `|` makes it clear that it has a continuation 
> line (and if you type the first line with `|`, your shell will wait for the 
> next line)
Ah now I understand. Yes that looks clearer so I've done that too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-06-15 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Looks great! Please give other reviews a day or two to respond in case they 
have comments.




Comment at: clang/test/Driver/program-path-priority.c:22
+/// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO_NOTREAL_GCC %s

Nit: this may be the preference of some reviewers, but they tend to favor `| \` 
in the end of the last line. `|` makes it clear that it has a continuation line 
(and if you type the first line with `|`, your shell will wait for the next 
line)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-06-15 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett marked 4 inline comments as done.
DavidSpickett added inline comments.



Comment at: clang/test/Driver/program-path-priority.c:20
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NO_NOTREAL_GCC %s

MaskRay wrote:
> Place `\` on the previous line to emphasize it has a continuation. Add two 
> spaces before the continuation line to emphasize it is a continuation line.
I think I've done this but I didn't quite understand the `\` part of the 
comment. The first lines already have one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-06-15 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett updated this revision to Diff 270716.
DavidSpickett added a comment.

Address comments from MaskRay.

- Use triple slash for actual comments
- symlink clang, don't run test on Windows
- Merged some commands onto one line e.g. touch/chmod


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c
  clang/test/lit.cfg.py

Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -46,6 +46,8 @@
 config.substitutions.append(
 ('%src_include_dir', config.clang_src_dir + '/include'))
 
+config.substitutions.append(
+('%target_triple', config.target_triple))
 
 # Propagate path to symbolizer for ASan/MSan.
 llvm_config.with_system_environment(
Index: clang/test/Driver/program-path-priority.c
===
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,106 @@
+/// Don't create symlinks on Windows
+// UNSUPPORTED: system-windows
+
+/// Check the priority used when searching for tools
+/// Names and locations are usually in this order:
+/// -tool, tool, -tool
+/// program path, PATH
+/// (from highest to lowest priority)
+/// A higher priority name found in a lower priority
+/// location will win over a lower priority name in a
+/// higher priority location.
+/// Prefix dirs (added with -B) override the location,
+/// so only name priority is accounted for, unless we fail to find
+/// anything at all in the prefix.
+
+/// Copy clang to a new dir which will be its
+/// "program path" for these tests
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: ln -s %clang %t/clang
+
+/// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: /gcc
+
+/// -gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+/// -gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+
+/// -gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+
+/// On program path, -gcc is preferred to plain gcc
+// RUN: touch %t/gcc && chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: /gcc
+
+/// -gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+
+/// -gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc && chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+
+/// -gcc has lowest priority so -gcc
+/// on PATH beats default triple in program path
+// RUN: touch %t/%target_triple-gcc && chmod +x %t/%target_triple-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
+// DEFAULT_TRIPLE_GCC: env/notreal-none-elf-gcc
+
+/// plain gcc on PATH beats default triple in program path
+// RUN: rm %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
+// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+
+/// default triple only chosen when no others are present
+// RUN: rm %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN:   | FileCheck

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-06-06 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/test/Driver/program-path-priority.c:15
+// "program path" for these tests
+// RUN: rm -rf %t
+// RUN: mkdir -p %t

`rm -rf %t && mkdir -p %t`
You can do this on one line



Comment at: clang/test/Driver/program-path-priority.c:17
+// RUN: mkdir -p %t
+// RUN: cp %clang %t
+

clang can be of several hundred bytes. Copying it is expensive. Use `ln -s` and 
add `UNSUPPORTED: system-windows` with a comment saying "Don't create symlinks 
on Windows" or similar



Comment at: clang/test/Driver/program-path-priority.c:19
+
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \

You can use `/// ` to make comments stand out from RUN and CHECK lines.



Comment at: clang/test/Driver/program-path-priority.c:20
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NO_NOTREAL_GCC %s

Place `\` on the previous line to emphasize it has a continuation. Add two 
spaces before the continuation line to emphasize it is a continuation line.



Comment at: clang/test/Driver/program-path-priority.c:39
+// RUN: | FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env{{/|}}notreal-none-elf-gcc
+

Just use `/` since you are going to use `ln -s` `chmod +x` etc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-22 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett added a comment.

Comments on cfe-dev 
(http://lists.llvm.org/pipermail/cfe-dev/2020-May/065432.html) also in favour 
of removing the default triple lookup. I tend to agree but think it makes more 
sense to land this first and address it in a follow up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-22 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett updated this revision to Diff 265694.
DavidSpickett added a comment.

- Addressed comments from nickdesaulniers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c
  clang/test/lit.cfg.py

Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -46,6 +46,8 @@
 config.substitutions.append(
 ('%src_include_dir', config.clang_src_dir + '/include'))
 
+config.substitutions.append(
+('%target_triple', config.target_triple))
 
 # Propagate path to symbolizer for ASan/MSan.
 llvm_config.with_system_environment(
Index: clang/test/Driver/program-path-priority.c
===
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,112 @@
+// Check the priority used when searching for tools
+// Names and locations are usually in this order:
+// -tool, tool, -tool
+// program path, PATH
+// (from highest to lowest priority)
+// A higher priority name found in a lower priority
+// location will win over a lower priority name in a
+// higher priority location.
+// Prefix dirs (added with -B) override the location,
+// so only name priority is accounted for, unless we fail to find
+// anything at all in the prefix.
+
+// Copy clang to a new dir which will be its
+// "program path" for these tests
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: cp %clang %t
+
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: {{/|}}gcc
+
+// -gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+// -gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc
+// RUN: chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env{{/|}}notreal-none-elf-gcc
+
+// -gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env{{/|}}notreal-none-elf-gcc
+
+// On program path, -gcc is preferred to plain gcc
+// RUN: touch %t/gcc
+// RUN: chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: {{/|}}gcc
+
+// -gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: env{{/|}}notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: {{/|}}gcc
+
+// -gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc
+// RUN: chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: env{{/|}}notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: {{/|}}gcc
+
+// -gcc has lowest priority
+// RUN: touch %t/%target_triple-gcc
+// RUN: chmod +x %t/%target_triple-gcc
+
+// -gcc on PATH beats default triple in program path
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
+// DEFAULT_TRIPLE_GCC: env{{/|}}notreal-none-elf-gcc
+
+// plain gcc on PATH beats default triple in program path
+// RUN: rm %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
+// DEFAULT_TRIPLE_NO_NOTREAL: env{{/|}}gcc
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+
+// default triple only chosen when no others are present
+// RUN: rm %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
+// DEFAULT_TRIPLE_NO_OTHERS: -gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc
+//

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-21 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4730
+  return std::string(P.str());
+  }
 } else {

style nit: are the `{}` on the `for` necessary?



Comment at: clang/test/Driver/program-path-priority.c:3
+// Names and locations are usually in this order:
+// -tool, tool, -tool
+// program path, PATH

s/defaul/default/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-15 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: clang/test/Driver/program-path-priority.c:72
+// -gcc has lowest priority
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc

DavidSpickett wrote:
> Pretty sure I'm stretching the limits here, probably not suitable for 
> Windows. I hoped to be able to capture the default triple in a CHECK line, 
> then use it in a following RUN line. (though thinking about how FileCheck 
> works, that isn't possible)
Resolved by adding a substitution for the default triple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-15 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett updated this revision to Diff 264201.
DavidSpickett added a comment.

Added target triple as a substitution so the tests
don't have to grep for it.
(which I didn't manage to verify on Windows in any case)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c
  clang/test/lit.cfg.py

Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -46,6 +46,8 @@
 config.substitutions.append(
 ('%src_include_dir', config.clang_src_dir + '/include'))
 
+config.substitutions.append(
+('%target_triple', config.target_triple))
 
 # Propagate path to symbolizer for ASan/MSan.
 llvm_config.with_system_environment(
Index: clang/test/Driver/program-path-priority.c
===
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,112 @@
+// Check the priority used when searching for tools
+// Names and locations are usually in this order:
+// -tool, tool, -tool
+// program path, PATH
+// (from highest to lowest priority)
+// A higher priority name found in a lower priority
+// location will win over a lower priority name in a
+// higher priority location.
+// Prefix dirs (added with -B) override the location,
+// so only name priority is accounted for, unless we fail to find
+// anything at all in the prefix.
+
+// Copy clang to a new dir which will be its
+// "program path" for these tests
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: cp %clang %t
+
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: {{/|}}gcc
+
+// -gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+// -gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc
+// RUN: chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env{{/|}}notreal-none-elf-gcc
+
+// -gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env{{/|}}notreal-none-elf-gcc
+
+// On program path, -gcc is preferred to plain gcc
+// RUN: touch %t/gcc
+// RUN: chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: {{/|}}gcc
+
+// -gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: env{{/|}}notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: {{/|}}gcc
+
+// -gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc
+// RUN: chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: env{{/|}}notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: {{/|}}gcc
+
+// -gcc has lowest priority
+// RUN: touch %t/%target_triple-gcc
+// RUN: chmod +x %t/%target_triple-gcc
+
+// -gcc on PATH beats default triple in program path
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
+// DEFAULT_TRIPLE_GCC: env{{/|}}notreal-none-elf-gcc
+
+// plain gcc on PATH beats default triple in program path
+// RUN: rm %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
+// DEFAULT_TRIPLE_NO_NOTREAL: env{{/|}}gcc
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+
+// default triple only chosen when no others are present
+// RUN: rm %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
+//

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-15 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett updated this revision to Diff 264187.
DavidSpickett added a comment.

Update from arc to hopefully run harbormaster builds again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c

Index: clang/test/Driver/program-path-priority.c
===
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,115 @@
+// Check the priority used when searching for tools
+// Names and locations are usually in this order:
+// -tool, tool, -tool
+// program path, PATH
+// (from highest to lowest priority)
+// A higher priority name found in a lower priority
+// location will win over a lower priority name in a
+// higher priority location.
+// Prefix dirs (added with -B) override the location,
+// so only name priority is accounted for, unless we fail to find
+// anything at all in the prefix.
+
+// Copy clang to a new dir which will be its
+// "program path" for these tests
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: cp %clang %t
+
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: {{/|}}gcc
+
+// -gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+// -gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc
+// RUN: chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env{{/|}}notreal-none-elf-gcc
+
+// -gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env{{/|}}notreal-none-elf-gcc
+
+// On program path, -gcc is preferred to plain gcc
+// RUN: touch %t/gcc
+// RUN: chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: {{/|}}gcc
+
+// -gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: env{{/|}}notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: {{/|}}gcc
+
+// -gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc
+// RUN: chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: env{{/|}}notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: {{/|}}gcc
+
+// -gcc has lowest priority
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+
+// -gcc on PATH beats default triple in program path
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
+// DEFAULT_TRIPLE_GCC: env{{/|}}notreal-none-elf-gcc
+
+// plain gcc on PATH beats default triple in program path
+// RUN: rm %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
+// DEFAULT_TRIPLE_NO_NOTREAL: env{{/|}}gcc
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+
+// default triple only chosen when no others are present
+// RUN: rm %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
+// DEFAULT_TRIPLE_NO_OTHERS: -gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: {{/|}}gcc
+
+// -B paths are searched separately so default triple will win
+// if put in one of those even if other paths have higher priority names
+// RUN: mkdir -p %t/prefix
+// RUN: mv %t/${default_triple}-gcc %t/prefix
+// RUN: touch

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-13 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett updated this revision to Diff 263696.
DavidSpickett added a comment.

- Updated test to look for forward or backslash in expected file paths.


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

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c

Index: clang/test/Driver/program-path-priority.c
===
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,115 @@
+// Check the priority used when searching for tools
+// Names and locations are usually in this order:
+// -tool, tool, -tool
+// program path, PATH
+// (from highest to lowest priority)
+// A higher priority name found in a lower priority
+// location will win over a lower priority name in a
+// higher priority location.
+// Prefix dirs (added with -B) override the location,
+// so only name priority is accounted for, unless we fail to find
+// anything at all in the prefix.
+
+// Copy clang to a new dir which will be its
+// "program path" for these tests
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: cp %clang %t
+
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: {{/|}}gcc
+
+// -gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+// -gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc
+// RUN: chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env{{/|}}notreal-none-elf-gcc
+
+// -gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env{{/|}}notreal-none-elf-gcc
+
+// On program path, -gcc is preferred to plain gcc
+// RUN: touch %t/gcc
+// RUN: chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: {{/|}}gcc
+
+// -gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: env{{/|}}notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: {{/|}}gcc
+
+// -gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc
+// RUN: chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: env{{/|}}notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: {{/|}}gcc
+
+// -gcc has lowest priority
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+
+// -gcc on PATH beats default triple in program path
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
+// DEFAULT_TRIPLE_GCC: env{{/|}}notreal-none-elf-gcc
+
+// plain gcc on PATH beats default triple in program path
+// RUN: rm %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
+// DEFAULT_TRIPLE_NO_NOTREAL: env{{/|}}gcc
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+
+// default triple only chosen when no others are present
+// RUN: rm %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
+// DEFAULT_TRIPLE_NO_OTHERS: -gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: {{/|}}gcc
+
+// -B paths are searched separately so default triple will win
+// if put in one of those even if other paths have higher priority names
+// RUN: mkdir -p %t/prefix
+// RUN: mv %t/${default_triple}-gcc %t/prefix
+// RUN: touch %t/notreal-none-elf-gcc
+//

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-13 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett updated this revision to Diff 263686.
DavidSpickett added a comment.

- Fix spelling
- Rework explanatory comments to be a bit clearer.


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

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c

Index: clang/test/Driver/program-path-priority.c
===
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,115 @@
+// Check the priority used when searching for tools
+// Names and locations are usually in this order:
+// -tool, tool, -tool
+// program path, PATH
+// (from highest to lowest priority)
+// A higher priority name found in a lower priority
+// location will win over a lower priority name in a
+// higher priority location.
+// Prefix dirs (added with -B) override the location,
+// so only name priority is accounted for, unless we fail to find
+// anything at all in the prefix.
+
+// Copy clang to a new dir which will be its
+// "program path" for these tests
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: cp %clang %t
+
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: /gcc
+
+// -gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+// -gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc
+// RUN: chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+
+// -gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+
+// On program path, -gcc is preferred to plain gcc
+// RUN: touch %t/gcc
+// RUN: chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: /gcc
+
+// -gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: /env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+
+// -gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc
+// RUN: chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: /env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+
+// -gcc has lowest priority
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+
+// -gcc on PATH beats default triple in program path
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
+// DEFAULT_TRIPLE_GCC: env/notreal-none-elf-gcc
+
+// plain gcc on PATH beats default triple in program path
+// RUN: rm %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
+// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+
+// default triple only chosen when no others are present
+// RUN: rm %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
+// DEFAULT_TRIPLE_NO_OTHERS: -gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: /gcc
+
+// -B paths are searched separately so default triple will win
+// if put in one of those even if other paths have higher priority names
+// RUN: mkdir -p %t/prefix
+// RUN: mv %t/${default_triple}-gcc %t/prefix
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s -B

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-13 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett created this revision.
DavidSpickett added reviewers: rogfer01, ddunbar, chandlerc.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.
DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: clang/test/Driver/program-path-priority.c:72
+// -gcc has lowest priority
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc

Pretty sure I'm stretching the limits here, probably not suitable for Windows. 
I hoped to be able to capture the default triple in a CHECK line, then use it 
in a following RUN line. (though thinking about how FileCheck works, that isn't 
possible)


As seen in:
https://bugs.llvm.org/show_bug.cgi?id=45693

When clang looks for a tool it has a set of
possible names for it, in priority order.
Previously it would look for these names in
the program path. Then look for all the names
in the PATH.

This means that aarch64-none-elf-gcc on the PATH
would lose to gcc in the program path.
(which was /usr/bin in the bug's case)

This changes that logic to search each name in both
possible locations, then move to the next name.
Which is more what you would expect to happen when
using a non default triple.

(-B prefixes maybe should follow this logic too,
but are not changed in this patch)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c

Index: clang/test/Driver/program-path-priority.c
===
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,113 @@
+// Check the priority used when searching for tools
+// Names and locations have a set priority. (highest to lowest)
+// -tool, tool, -tool
+// program path, PATH
+// Such that a more specific name found in the PATH
+// will win over a less specific name found in the program path
+// Prefix dirs (added with -B) overrride the location,
+// so only name priority is accounted for, unless we fail to find
+// anything at all in the prefix.
+
+// Copy clang to a new dir which will be its
+// "program path" for these tests
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: cp %clang %t
+
+// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: /gcc
+
+// -gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+// -gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc
+// RUN: chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+
+// -gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc
+// RUN: chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+
+// On program path, -gcc is preferred to plain gcc
+// RUN: touch %t/gcc
+// RUN: chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: /gcc
+
+// -gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: /env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+
+// -gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc
+// RUN: chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 \
+// RUN: | FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: /env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+
+// -gcc has lowest priority
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+
+// -gcc on PATH beats default triple in program path
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+// RUN: env "PATH=%t/env/" %t/clang -###

[PATCH] D79842: [clang][Driver] Correct tool search path priority

2020-05-13 Thread David Spickett via Phabricator via cfe-commits 
DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: clang/test/Driver/program-path-priority.c:72
+// -gcc has lowest priority
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc

Pretty sure I'm stretching the limits here, probably not suitable for Windows. 
I hoped to be able to capture the default triple in a CHECK line, then use it 
in a following RUN line. (though thinking about how FileCheck works, that isn't 
possible)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits