Re: [PATCH] D12466: Fix empty -L Path on OSX hosts

[Martell Malone via cfe-commits]

martell added a comment.

> Is this testable?


Seems to me the only way to add this as a test would be to add bad code to 
clang to purposely pass an empty LibPath argument.
This is probably something we do not want to do.

This is probably why Reid never asked me for one in the review.


Repository:
  rL LLVM

http://reviews.llvm.org/D12466



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

Re: [PATCH] D12466: Fix empty -L Path on OSX hosts

[Martell Malone via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL254117: Driver: protect from empty -L args (authored by 
martell).

Changed prior to commit:
  http://reviews.llvm.org/D12466?vs=33510=41202#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D12466

Files:
  cfe/trunk/lib/Driver/ToolChain.cpp

Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -619,7 +619,8 @@
 void ToolChain::AddFilePathLibArgs(const ArgList ,
ArgStringList ) const {
   for (const auto  : getFilePaths())
-CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + LibPath));
+if(LibPath.length() > 0)
+  CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + LibPath));
 }
 
 void ToolChain::AddCCKextLibArgs(const ArgList ,


Index: cfe/trunk/lib/Driver/ToolChain.cpp
===
--- cfe/trunk/lib/Driver/ToolChain.cpp
+++ cfe/trunk/lib/Driver/ToolChain.cpp
@@ -619,7 +619,8 @@
 void ToolChain::AddFilePathLibArgs(const ArgList ,
ArgStringList ) const {
   for (const auto  : getFilePaths())
-CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + LibPath));
+if(LibPath.length() > 0)
+  CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + LibPath));
 }
 
 void ToolChain::AddCCKextLibArgs(const ArgList ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12466: Fix empty -L Path on OSX hosts

[David Blaikie via cfe-commits]

Is this testable?
On Nov 25, 2015 5:56 PM, "Martell Malone via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL254117: Driver: protect from empty -L args (authored by
> martell).
>
> Changed prior to commit:
>   http://reviews.llvm.org/D12466?vs=33510=41202#toc
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D12466
>
> Files:
>   cfe/trunk/lib/Driver/ToolChain.cpp
>
> Index: cfe/trunk/lib/Driver/ToolChain.cpp
> ===
> --- cfe/trunk/lib/Driver/ToolChain.cpp
> +++ cfe/trunk/lib/Driver/ToolChain.cpp
> @@ -619,7 +619,8 @@
>  void ToolChain::AddFilePathLibArgs(const ArgList ,
> ArgStringList ) const {
>for (const auto  : getFilePaths())
> -CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + LibPath));
> +if(LibPath.length() > 0)
> +  CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + LibPath));
>  }
>
>  void ToolChain::AddCCKextLibArgs(const ArgList ,
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12466: Fix empty -L Path on OSX hosts

[David Blaikie via cfe-commits]

dblaikie added a subscriber: dblaikie.
dblaikie added a comment.

Is this testable?


Repository:
  rL LLVM

http://reviews.llvm.org/D12466



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

Re: [PATCH] D12466: Fix empty -L Path on OSX hosts

[Reid Kleckner via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

This looks fine, just make sure the indentation is right. Phab seems confused.


Repository:
  rL LLVM

http://reviews.llvm.org/D12466



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

Re: [PATCH] D12466: Fix empty -L Path on OSX hosts

[Yaron Keren via cfe-commits]

yaron.keren added a comment.

I have never used OSX, Try to add one of the Apple clang developers as 
reviewers, they know more than me about OSX.

My guess there should not be empty paths in  in TC.getFilePaths(). That patch 
seems only to sidestep the issue which will probably surface in other places. 
Can you debug why this empty path is created and fix that?


Repository:
  rL LLVM

http://reviews.llvm.org/D12466



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