Replies: 2 comments 1 reply
-
My gut instinct would be to go with a combination of 1 and 3. In fact I'm pretty sure 3 has been requested before, and I thought that at least at one point we were already doing 1. Either way, those two options feel solidly justifiable to me. |
Beta Was this translation helpful? Give feedback.
1 reply
-
No, and I don't understand why you would think so. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Am I right in believing that the only way to explicitly specify system include directories correctly (so we end up using
-isystem somedir
instead of-I somedir
) is to have to wrap up the directories ininclude_directories('somedir', is_system: true)
? E.g. -The (admittely quite minor) problem with that is that use of the
include_directories(...)
function blindly doubles up all such include dirs passed to the compile command line. I think the idea is that the user will automatically be given a source-relative include path as well as a path relative to the build directory.I'm not sure why it's done like this; not convinced this is a good idea. I can't think of a scenario where the build-script-writing user does not always know whether they want to add source-relative include directories or build-relative ones, but there doesn't seem to be a way for the user to choose: If you need to add a build-relative include path for compiling some source that depends on generated headers somewhere under the build dir, it seems as though
include_directories('path/under/builddir')
is the thing to use but your compilation will now be supplied with the desired-I [buildroot]/path/under/builddir
but also needlessly with-I [srcroot]/path/under/builddir
.Similarly, if the user just wants to specify system include dirs ( these may or may not be an absolute path, which the include_directories code expects and allows), then using this needlessly doubles up with a pointless duplicate thrown at the compiler.
I haven't tested the impact on performance, which may depend on the example used. I'd guess it's likely to be negigible but there's also a chance that, for certain projects, it might be small but not insignificant. However, needlessly doubling up on some include search dirs is never going to be faster than using only what you need. It also seems a little sloppier than it could be, compared to using only and exactly the search dirs that the user knows they want/need, and so adds potential for some unnecessary problems (e.g. generating a header under a builddir that ends up unnecessarily conflicting with a src dir header in a directory that the user doesn't intend be searched but which was forced on them through use of
include_directories(...)
).In short, if the user can help out both build performance, correctness/behaviour through stricter use, and clarity/comprehension for the reader, then there should be a simple enough way for the user to specify that.
What are people's thoughts on the following? -
include_directories(...)
, but, internally, we can at least trivially prevent needless duplication of absolute paths. Currentlysys_incs = include_directories('c:/some/path/to/custom/platform/headers', is_system: true)
ultimately ends up usingcl.exe ... /clang:-isystem c:/some/path/to/custom/platform/headers /clang:-isystem c:/some/path/to/custom/platform/headers ...
, so simply checking if the src-relative and build-relative paths are either exactly equal or an absolute path, then we just use one.include_directories(...)
could take new parameters (with defaults resulting in current auto-doubling-up behaviour) to allow the user to say they just want the src-relative or build-relative path used for include directories. However, use of a either 'builddir-relative' flag or a 'srcdir-relative' flag might be slightly confusing to the user if they're passing absolute paths.src_relative_include_directories(...)
,build_relative_include_directories(...)
.Beta Was this translation helpful? Give feedback.
All reactions