pirama marked 2 inline comments as done.
pirama added inline comments.

================
Comment at: lib/Driver/Tools.cpp:289
+      CmdArgs.push_back("-rpath");
+      CmdArgs.push_back(Args.MakeArgString(CandidateRPath.c_str()));
+    }
----------------
rnk wrote:
> We shouldn't add rpath to every binary linked by clang. We should only add 
> rpath when we know the user is linking against a shared library provided by 
> clang. See for example the way the darwin toolchain handles this in 
> Toolchains.cpp.
> 
> Right now most libraries provided by clang are static, so there hasn't been 
> much need to add rpath, but if we add more shared libraries (xray, 
> sanitizers, profiling, etc), then I think we'll want to do this along with 
> adding an option to suppress the implicit addition of rpath.
PTAL.


================
Comment at: test/Driver/arch-specific-libdir-rpath.c:18
+//
+// CHECK-ARCHDIR: 
-L{{.*}}/Inputs/resource_dir_with_arch_subdir/lib/linux{{.*}} "-rpath" 
{{.*}}/Inputs/resource_dir_with_arch_subdir/lib/linux
+// CHECK-NO-ARCHDIR-NOT: -L{{.*}}Inputs/resource_dir
----------------
pirama wrote:
> mgorny wrote:
> > pirama wrote:
> > > Hahnfeld wrote:
> > > > Can you split that into two lines? Then it won't fail if there is some 
> > > > argument added in between
> > > Splitting into two lines makes FileCheck eagerly match the arch-subdir 
> > > and -rpath into the {{.*}} next to the "-L".  This causes the check for 
> > > -rpath to fail.
> > > 
> > > The test accepts intermediate arguments because of the wildcard right 
> > > after the -L...Inputs/...
> > Please do not rely on implicit assumptions like that. One day someone may 
> > decide to 'fix' the wildcard not to match whitespace, and make the wrong 
> > assumption about order. If for anything, splitting in two would make this 
> > more readable.
> Do you have any suggestions?  Right now, if I split it into two lines, the 
> second check fails due to the eager regex match.
Done.  I was able to get the exact path from the -cc1 invocation and use it to 
remove the wildcards.


https://reviews.llvm.org/D30015



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

Reply via email to