Meson: System headers should be included with -isystem rather than -I

[mcatanzaro]

All system header files should be included with -isystem rather than -I. The use-case is to have a replacement for the CMake statement include_directories(SYSTEM ...) which suppresses compiler warnings from header files. For instance, in WebKit we use this for all include directories other than WebKit's own. Without using -isystem, there is no convenient way to suppress compiler warnings from header files outside the user's control. Unfortunately, implementing might be awkward because it requires overriding -I flags that are hardcoded in pkg-config files with -isystem, which is not really how pkg-config is intended to be used.

Note that #747 was a step in this direction, but it didn't get merged.

[jpakkane]

There is some support for isystem in the code (you can grep for it) but it's not much. The obvious question here would be what sources of headers should be using isystem?

• those that come from system pkgconfig (highly suspicious)
• declare_dependencies that come from subprojects (because they are, in a way, from "the system")
• include_directories that the user has explicitly tagged with system
• others?

Could we take this further? As an example, could we suggest a way for upstream pkg-config to change the arguments? Like when a specific environment variable is set or due to a setting in a hypothetical .pkgconfig-config file?

[nirbheek]

I'm not sure we want _all_ declare_dependencies from subprojects being 'system' because at least in the gstreamer case we want C warnings from subprojects that are "ours" and not from those that are "third-party". So we probably want a way to set subprojects as being "third-party" or similar.

[martin-ejdestig]

This would help with getting clang-tidy to report errors only in header files that belongs to the project.

clang-tidy has an option for setting a regular expression for header filter but it is quite limited (limited regular expressions and gets confused easily by relative paths etc in the compile_commands.json). System headers are ignored by default though.

[jpakkane]

If you can specify a regex for include paths, then putting subprojects in it should make it ignore all subprojects, since they are guaranteed to have that somewhere in their path, be it source or build dir.

[martin-ejdestig]

As I said, the regex implementation seems to be quite limited (have not gotten "exclude path" to work) and it also gets confused by e.g. relative paths.

[martin-ejdestig]

Also, it is not possible to know what path(s) to exclude and enter in .clang-tidy or to pass on the command line to clang-tidy for external dependencies found with pkg-config.

[nirbheek]

Another reason why we want to do this is because currently you have to be careful while specifying library deps with includes to the dependencies : kwarg. You must list internal deps first, and then external deps, otherwise the compiler will look in the system include paths for headers of internal deps.

This is not a problem for library paths because we don't use -Lfoo -lbar for internal deps, and specify the full path to the shared library file instead.

[dcbaker]

We'd like this for mesa too, so I'll probably be looking at it soon. Does it makes sense to have a keyword arg for dependency and declare_dependency to override this behavior?

[jpakkane]

This is more difficult than it first seems. Changing -I to -isystem changes the order lookups are done in. It can (and already has) lead to nasty bugs.

[dcbaker]

For mesa a it would be sufficient to have an argument for declare_dependency to use -isystem instead of -I, would that in itself be a useful addition?

[jpakkane]

Not really, since include_directories already has it. :)

[dcbaker]

excellent, that was the easiest bug I've ever fixed :)

[bredelings]

Hi, newer GCC spits out a ton of -Wdeprecated-copy errors on eigen headers. My project includes an internal copy of eigen, but defaults to using the system eigen headers using dependency('eigen3') if possible. When using the internal copy, I can specify is_system:true to suppress the errors, but when using the pkg-config dependency the errors are not suppressed. This seems a bit unfortunate.

I'm wondering if there is anything that can be done. I understand the changing -I to -isystem automatically changes the search order instead of just suppressing errors. So maybe the problem is not solvable in such a simple fashion.

One possibility would be to add an is_system flag to dependency( ), which would explicitly replace -I with -isystem. In theory the user could choose this option to intentionally change both the errors and the search order, which they might want. However, I suppose performing this change is not guaranteed to work whenever pkg-config already works.

Looking further upstream, what would be necessary to solve this problem correctly?

• the compiler could have a -i_no_error flag which does not change the search order
• pkg-config could have a --suppress-error option which might change the results.
  Other thoughts?

[Jehan]

While cross-compiling pango with MinGW64, there are many warnings in MinGW system headers and pango sets a whole bunch of -Werror (in particular -Werror=redundant-decls is what prevents the cross-build from succeeding). Of course this should be reported to MinGW64 project too, but in the meantime, it is of interest to know if we can build with meson with -isystem (as I understand, it would not choke on warnings from what is considered "system headers" even with the corresponding -Werror) for such files.

So this report is about this exact kind of issue, right? Is there any way currently (maybe a complicated one) or we have to wait for some 'is_system' flag to be implemented everywhere?

Also I did read that there is some basic implementation already, and I could grep the code for it, but I could not really figure it how it can be used currently.

See also: https://gitlab.gnome.org/GNOME/pango/merge_requests/41

[gerion0]

LLVM libraries have the same problems. I've seen that this pull request for boost was merged long ago. One could add the same solution for llvm. However, I think a generic is_system option for dependency() would be more user friendly.

As an example, one warning caused by LLVM:

/usr/lib64/llvm/7/include/llvm/ADT/DenseMap.h:530:45: Warnung: unverwendeter Parameter »Key« [-Wunused-parameter]
   BucketT *InsertIntoBucketImpl(const KeyT &Key, const LookupKeyT &Lookup,
                                 ~~~~~~~~~~~~^~~
/usr/lib64/llvm/7/include/llvm/ADT/DenseMap.h: In Instanziierung von »BucketT* llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::InsertIntoBucketImpl(const KeyT&, const LookupKeyT&, BucketT*) [with LookupKeyT = llvm::Function*; DerivedT = llvm::DenseMap<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > >, llvm::DenseMapInfo<llvm::Function*>, llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > > >; KeyT = llvm::Function*; ValueT = std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > >; KeyInfoT = llvm::DenseMapInfo<llvm::Function*>; BucketT = llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > >]«:
/usr/lib64/llvm/7/include/llvm/ADT/DenseMap.h:512:15:   erfordert durch »BucketT* llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::InsertIntoBucket(BucketT*, KeyArg&&, ValueArgs&& ...) [with KeyArg = llvm::Function*; ValueArgs = {}; DerivedT = llvm::DenseMap<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > >, llvm::DenseMapInfo<llvm::Function*>, llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > > >; KeyT = llvm::Function*; ValueT = std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > >; KeyInfoT = llvm::DenseMapInfo<llvm::Function*>; BucketT = llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > >]«
/usr/lib64/llvm/7/include/llvm/ADT/DenseMap.h:304:12:   erfordert durch »llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::value_type& llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::FindAndConstruct(KeyT&&) [with DerivedT = llvm::DenseMap<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > >, llvm::DenseMapInfo<llvm::Function*>, llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > > >; KeyT = llvm::Function*; ValueT = std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > >; KeyInfoT = llvm::DenseMapInfo<llvm::Function*>; BucketT = llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > >; llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::value_type = llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > >]«
/usr/lib64/llvm/7/include/llvm/ADT/DenseMap.h:308:45:   erfordert durch »ValueT& llvm::DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT>::operator[](KeyT&&) [with DerivedT = llvm::DenseMap<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > >, llvm::DenseMapInfo<llvm::Function*>, llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > > >; KeyT = llvm::Function*; ValueT = std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > >; KeyInfoT = llvm::DenseMapInfo<llvm::Function*>; BucketT = llvm::detail::DenseMapPair<llvm::Function*, std::__cxx11::list<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > >, std::allocator<std::pair<llvm::AnalysisKey*, std::unique_ptr<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>, std::default_delete<llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator> > > > > > >]«
/usr/lib64/llvm/7/include/llvm/IR/PassManager.h:863:60:   erfordert durch »llvm::AnalysisManager<IRUnitT, ExtraArgTs>::ResultConceptT& llvm::AnalysisManager<IRUnitT, ExtraArgTs>::getResultImpl(llvm::AnalysisKey*, IRUnitT&, ExtraArgTs ...) [with IRUnitT = llvm::Function; ExtraArgTs = {}; llvm::AnalysisManager<IRUnitT, ExtraArgTs>::ResultConceptT = llvm::detail::AnalysisResultConcept<llvm::Function, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>]«
/usr/lib64/llvm/7/include/llvm/IR/PassManager.h:691:9:   erfordert durch »typename PassT::Result& llvm::AnalysisManager<IRUnitT, ExtraArgTs>::getResult(IRUnitT&, ExtraArgTs ...) [with PassT = llvm::TargetLibraryAnalysis; IRUnitT = llvm::Function; ExtraArgTs = {}; typename PassT::Result = llvm::TargetLibraryInfo]«
/usr/lib64/llvm/7/include/llvm/Analysis/AliasAnalysis.h:1024:51:   von hier erfordert

[Trass3r]

For mesa a it would be sufficient to have an argument for declare_dependency to use -isystem instead of -I, would that in itself be a useful addition?

Not really, since include_directories already has it. :)

Should be documented: https://mesonbuild.com/Include-directories.html

[1ace]

Any progress on this?

I'm hitting the same issue with warnings in some LLVM versions preventing me from adding the warning flags to anything that uses LLVM headers in Mesa...

dependency(..., is_system: true) generating -isystem instead of -I would trivially fix the issue.

[ailin-nemui]

you can get a list of directories from

clang++ -Wp,-v -xc++ -fsyntax-only - </dev/null

then just exclude any -I that reference paths in this list, from being turned into -isystem (as that might lead to the search order problem)

[dankegel]

This is really annoying. All my projects that use -Werror -Wsuggest-override had to stop doing that when I ported them to meson, because system boost has tons of those warnings, and because my 'system' boost is in /opt/boost (so the fix for https://github.com/mesonbuild/meson/issues/1569 didn't help). So it's not just pkgconfig dependencies, it's not just /usr/include, and it's definitely not just manual include directories.

The default behavior for system dependencies of any flavor should be to lower the error level, and the notion of what is system should be flexible.

As usual, though, here be dragons. I had to stop using -isystem in one situation recently on Mac.
Turns out /usr/local/include is on the default include path,
and -isystem /opt/blah/include causes /opt/blah to be searched
after /usr/local/include.

So don't assume that -isystem is necessarily the fix.

[Hi-Angel]

As usual, though, here be dragons. I had to stop using -isystem in one situation recently on Mac.
Turns out /usr/local/include is on the default include path,
and -isystem /opt/blah/include causes /opt/blah to be searched
_after_ /usr/local/include.

So don't assume that -isystem is necessarily the fix.

Order shouldn't matter, also headers are usually protected with guards.

[dankegel]

Order should matter

Tell that to my broken build. If there are multiple versions of a library on the system (say, one for the system, and one for a platform you're building for), order most definitely does matter.

headers are usually protected with guards

Doesn't help if you're including the wrong version of the headers.

[gerion0]

This is more difficult than it first seems. Changing -I to -isystem changes the order lookups are done in. It can (and already has) lead to nasty bugs.

What I found in the GCC documentation for that is:

The lookup order is as follows:

1. For the quote form of the include directive, the directory of the current file is searched first.
2. For the quote form of the include directive, the directories specified by -iquote options are searched in left-to-right order, as they appear on the command line.
3. Directories specified with -I options are scanned in left-to-right order.
4. Directories specified with -isystem options are scanned in left-to-right order.
5. Standard system directories are scanned.
6. Directories specified with -idirafter options are scanned in left-to-right order.

I have searched a little bit but mainly found bugs containing -isystem /usr/include (example) and as far as I can tell not related to Meson. Can you explain a little bit more what bugs can arise? Where is the best starting place to implement a is_system: true feature?

[ailin-nemui]

the stack overflow post you linked seems like a good explanation as well as containing an algorithm to avoid modifying the existing -isystem search path.

ideally, meson should use -isystem for all include directories supplied through dependencies, because that will give the benefit of higher warnings and failure flags for your own headers

however, in that case you have to be careful of not simply running s/-I/-isystem/ as unconditionally that might mess up the search path, for example if the original dependency pkgconfig file specifies Cflags such as -I/usr/local/include

in summary, paths given in -I that are already * in the built in -isystem list *must not be turned into -isystem

[dankegel]

Second caveat: -isystem can explode on the mac.

As I mentioned above, /usr/local/include is on the default include path,
and -isystem /opt/blah/include causes /opt/blah to be searched
after /usr/local/include. On the Mac, where just using brew to install
anything causes /usr/local/include to be populated with everything under the sun,
with different versions than you installed into your own ecosystem,
so it's somewhat fatal there to take a .pc file that worked just fine with -I
and transmogrify it to use -isystem.

[NickeZ]

and -isystem /opt/blah/include causes /opt/blah to be searched after /usr/local/include.

Could you elaborate on that? In my own testing, with gcc and clang on linux, this isn't true. And according to the docs it isn't true.

[NickeZ]

My experiment is here: https://github.com/NickeZ/sys-include-order

I've named the header stdint.h to show that if the standard include paths come before the command line supplied include paths it won't build.

My experiment clearly shows that -isystem . makes . come before the built-in paths. If you remove that argument from the makefile it will fail to build.

$ ./build.sh
+ CC=gcc
+ make clean all
rm -f main main.o fun.o
gcc -Wall -Wextra -Wpedantic -std=c11 -isystem .   -c -o main.o main.c
gcc -Wall -Wextra -Wpedantic -std=c11 -isystem .   -c -o fun.o fun.c
gcc   main.o fun.o   -o main
+ CC=clang
+ make clean all
rm -f main main.o fun.o
clang -Wall -Wextra -Wpedantic -std=c11 -isystem .   -c -o main.o main.c
clang -Wall -Wextra -Wpedantic -std=c11 -isystem .   -c -o fun.o fun.c
clang   main.o fun.o   -o main

edit: Also critically, if you change it to -idirafter it doesn't build.

[jpakkane]

As a clarification, if a dependency defines its include args with -I then we can _not_ automatically convert them to -isystem because there is no way to do it reliably. There are cases where doing that will break things in a way that can't be fixed otherwise. Thus we'd need to, at the very least, need a way to disable this conversion at which point it becomes roughly identical to an option to convert them to system includes.

Now there is a way to fix it properly, and that is Flatpak or some similar sandboxing mechanism that ensures that there is only one set of headers for any dependency. Sadly this won't work on Windows or Mac until they add corresponding functionality, and it is entirely possible that they will never do that.

[NickeZ]

There are cases where doing that will break things in a way that can't be fixed otherwise.

I think this only applies to if you have -isystem directories that also already are built-in standard directories. Otherwise it should be perfectly safe to convert -I to -isystem.

[dankegel]

NickeZ's test case made me revisit my mac build problems. I retract my claims, and replace them with vague unease :-)

I tested by changing -I to -isystem in all my non-system .pc files, and building stuff with brew ninja 1.9.0. So far, I don't have any minimal test cases showing -isystem problems, as long as you omit the space between -isystem and the path in question in your .pc file. (The pkgconfig I'm using, pkgconf 0.28, is broken if you give a space between -isystem and the path.)

Two other not-very-useful observations:
1) /usr/local/include definitely takes priority over /usr/include there by default.
2) my app builds properly on mac unless boost from brew is installed; the build system is cmake. In the bad case, cmake finds the include files where I intend (/opt/me/foo/include), but finds the library files in the wrong place (/usr/local/lib). No idea whose fault that is.

[NickeZ]

@dankegel you can check which include directores are the standard ones with:

gcc -xc -E -v - < /dev/null 2>&1 | grep '^ '

[gerion0]

I think, this bug can be closed now since #5953 is merged.

[mcatanzaro]

Yay!