[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)
dlj added a comment.
In https://reviews.llvm.org/D30372#688154, @mehdi_amini wrote:
> In https://reviews.llvm.org/D30372#687060, @ahatanak wrote:
>
> > In https://reviews.llvm.org/D30372#687054, @dlj wrote:
> >
> > > In https://reviews.llvm.org/D30372#686871, @ahatanak wrote:
> > >
> > > > Have you considered using "include_directories" in CMakeLists.txt to
> > > > avoid including "../Something.h"?
> > >
> > >
> > > I don't want to take that approach, and there's a specific reason why: my
> > > concern isn't with three extra bytes, it's the fact that the headers are
> > > in an unusual place. It's typical to include headers from a subdirectory,
> > > or from the includes directory, but far less common to include from a
> > > parent lib directory. Hiding that fact seems strictly worse to me.
> >
> >
> > OK. I was suggesting that only because LLVM already uses
> > include_directories to include a header that exists in the parent directory
> > in some cases. Apparently that's not what you wanted.
>
>
> Yes it seems "common" in LLVM to do this in CMake: `include_directories(
> ${CMAKE_CURRENT_BINARY_DIR}/.. ${CMAKE_CURRENT_SOURCE_DIR}/.. )`
Actually, since I'm still keeping these files in the clangDriver rule, the
lib/Driver directory is already on the search path, so I can use a subpath of
Driver already.
https://reviews.llvm.org/D30372
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)
mehdi_amini added a comment.
In https://reviews.llvm.org/D30372#687060, @ahatanak wrote:
> In https://reviews.llvm.org/D30372#687054, @dlj wrote:
>
> > In https://reviews.llvm.org/D30372#686871, @ahatanak wrote:
> >
> > > Have you considered using "include_directories" in CMakeLists.txt to
> > > avoid including "../Something.h"?
> >
> >
> > I don't want to take that approach, and there's a specific reason why: my
> > concern isn't with three extra bytes, it's the fact that the headers are in
> > an unusual place. It's typical to include headers from a subdirectory, or
> > from the includes directory, but far less common to include from a parent
> > lib directory. Hiding that fact seems strictly worse to me.
>
>
> OK. I was suggesting that only because LLVM already uses include_directories
> to include a header that exists in the parent directory in some cases.
> Apparently that's not what you wanted.
Yes it seems "common" in LLVM to do this in CMake: `include_directories(
${CMAKE_CURRENT_BINARY_DIR}/.. ${CMAKE_CURRENT_SOURCE_DIR}/.. )`
https://reviews.llvm.org/D30372
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)
ahatanak added a comment. In https://reviews.llvm.org/D30372#687054, @dlj wrote: > In https://reviews.llvm.org/D30372#686871, @ahatanak wrote: > > > Have you considered using "include_directories" in CMakeLists.txt to avoid > > including "../Something.h"? > > > I don't want to take that approach, and there's a specific reason why: my > concern isn't with three extra bytes, it's the fact that the headers are in > an unusual place. It's typical to include headers from a subdirectory, or > from the includes directory, but far less common to include from a parent lib > directory. Hiding that fact seems strictly worse to me. OK. I was suggesting that only because LLVM already uses include_directories to include a header that exists in the parent directory in some cases. Apparently that's not what you wanted. https://reviews.llvm.org/D30372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)
dlj added a comment. In https://reviews.llvm.org/D30372#686871, @ahatanak wrote: > Have you considered using "include_directories" in CMakeLists.txt to avoid > including "../Something.h"? I don't want to take that approach, and there's a specific reason why: my concern isn't with three extra bytes, it's the fact that the headers are in an unusual place. It's typical to include headers from a subdirectory, or from the includes directory, but far less common to include from a parent lib directory. Hiding that fact seems strictly worse to me. https://reviews.llvm.org/D30372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)
ahatanak added a comment. Have you considered using "include_directories" in CMakeLists.txt to avoid including "../Something.h"? https://reviews.llvm.org/D30372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
