[vlc-devel] [PATCH 4/4] seek config.h include via angled bracket search

[Lyndon Brown]

On Wed, 2020-10-07 at 08:36 +0200, Steve Lhomme wrote:
> On 2020-10-06 20:29, Lyndon Brown wrote:
> > On Tue, 2020-10-06 at 08:33 +0200, Steve Lhomme wrote:
> > >   > As I said before, my understanding of the difference between
> > > quoting
> > >   > and angle brackets is that with quoting, the directory of the
> > > file
> > >   > doing the include is checked first, before falling back to
> > > the
> > > angle
> > >   > bracket path search.
> > > 
> > > It seems more subtle than that and is left to compilers to
> > > decide.
> > > For
> > > example in gcc you have this:
> > > https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html
> > > 
> > > "Directories specified with -iquote apply only to the quote form
> > > of
> > > the
> > > directive, #include "file". Directories specified with -I,
> > > -isystem,
> > > or
> > > -idirafter apply to lookup for both the #include "file" and
> > > #include
> > > <file> directives."
> > > 
> > > Which is what should be used for local includes to make sure you
> > > don't
> > > include similar files from other places. That's probably
> > > overkill.
> > > 
> > > These are the rules it applies:
> > > 
> > > <snip>
> > 
> > Yes, I'd seen all of that, thanks though. Essentially it boils down
> > to
> > the same fundamental concept, with a mechanism available for
> > specifying
> > some extra quote-only search locations.
> 
> Not just that, I think it clarifies one fundamental difference
> between 
> "" and <>. "" is for files local to your project (current dir or
> project 
> dir) whereas <> is for external files.

Well yes, quotes provide a clean and simple way to get at local/private
headers within the source directories, relative to compilation targets.
It would be a mess to use <> and a set of src/* -I paths for each
target *.c file. <> allows us to skip that local search for situations
where it does not apply or is not suitable, especially with external
stuff.

> MSVC has similar concepts
> https://docs.microsoft.com/en-us/cpp/preprocessor/hash-include-directive-c-cpp?view=vs-2019
> 
> Here "" includes also look in upper directories before looking in -I 
> directories.

I think you might have misread it, or I'm misunderstanding your
reference to "upper directories". The whole "parent"/"grandparent"
stuff (step #2 of quoted form), is about nested includes. The point
itself is a little unclear about this, with another interpretation
possible, but the subsequent paragraphs only speak in terms of nesting.

So as an example, let's say we have:
 - includes/a.h
 - includes/b.h
    - containing `#include "a.h"`
 - includes/c.h
    - containing `#include <b.h>`
 - src/foo/bar.h
 - src/foo/bar.c
    - containing `#include <assert.h>`
    - containing `#include "bar.h"`
    - containing `#include "a.h"`
    - containing `#include "b.h"`
    - containing `#include <c.h>`
 - An -I path for 'includes' and -I path or whatever for system
includes.

The process for src/foo/bar.c would be:
(Where Q-x: quoted step, AB-x: angle bracket step from MSVC article)

1. <assert.h>
  - AB-1/2: Found in system path.
2. "bar.h"
  - Q-1: Found in src/foo/ (directory of bar.c).
3. "a.h"
  - Q-1: Not found in src/foo/ (directory of bar.c).
  - Q-2: This is not nesting, so skipped.
  - Q-3: Found in our 'includes' -I path.
4. "b.h"
  - Q-1: Not found in src/foo/.
  - Q-2: This is not nesting, so skipped.
  - Q-3: Found in our 'includes' -I path.
5. "a.h" (nested include of b.h)
  - Q-1: Skipped? Otherwise, not found in /src/foo.
  - Q-2: Found in includes/ from being directory of "parent" (b.h).
6. <c.h>
  - AB-1: Found in 'includes' -I path.
7. <b.h> (nested include of c.h)
  - AB-1: Found in 'includes' -I path.
8. "a.h" (nested include of b.h)
  - Q-1: Skipped? Otherwise, not found in /src/foo.
  - Q-2: Found in includes/ from being directory of "parent" (b.h). If
it had not been found, the next check would have been the directory of
"grandparent" (c.h), jumping backwards through the locations of the
nested includes towards bar.c.

(Include guards solve the problem of files included multiple times of
course).

Assuming includes/ is our first -I path, we can definitely optimise
this by either (a) having bar.c use "../../includes/x.h" and those
includes internally using <>, or (b) using <> everywhere for [a|b|c].h.
Only if a certain number of other -I paths are checked before
'includes' would it not be so optimal to do so, though there is also
the benefit of avoiding local name clashes in the <> use.

So in terms of a config.h in the build directory -I, we don't want Q-1
or Q-2 searches to take place, we want to jump straight to Q-3 (AB-1).
Ensuring we pick up the right config.h comes down to the order of -I
paths; We need to ensure the build directory -I comes before contrib
include path -I, should there be any danger of a contrib config.h
ending up in its include path (which would be a bad idea for it to do).

In terms of things like src/libvlc.h and src/foo/bar.h, the only reason
these work at all as "libvlc.h" and "foo/bar.h" or <libvlc.h> and
<foo/bar.h> is because src/ is an -I path, not because of some
directory traversal search from src/foo/bar/ upwards.

> There are 3 main categories of include dirs: the project ones, the 
> external ones and the system ones. What you are proposing is to
> consider 
> there's just one type and we will manage the order ourselves. It's 
> technically what we are doing today but I don't think it's the
> proper 
> way, nor a desirable direction.

The order is always managed ourselves by way of us giving -I paths and
in a certain order to the compiler in our build system code.

We always have to be cautious about what order we give those -I paths
in and what set of -I paths we use for each target, should there be any
chance of file name clashes. It's also worth considering for optimal
searching (preferring those with fewer items towards the end).

What I'm suggesting is this:
 - config.h: This exists in the build directory, which must be
considered an "external" path due to the possibility of out-of-source
builds. The Q-1 and Q-2 checks don't apply to it, they just add
pointless inefficiency. We should thus use <>.
 - includes/vlc_*.h:
   - Within these files, the benefits of switching to use of quotes is
arguable, but otherwise we should fix the small number of inconsistent
use of quotes with <> for consistency (and to avoid the pointless Q-1
check if that applies to nested cases).
   - Where included in *.c/*.cpp files, we should replace the few
inconsistent use of quotes with <>, to fully rely on AB-1, since Q-1
does not apply and creates pointless inefficiency. It would be a bad
idea IMO to switch to using direct Q-1 "../../includes/vlc_*.h" style
for them since this would be very messy and inflexible, especially for
plugins.
 - For private headers within src/, we should tidy up the mess of
different styles, and prefer use of quoted relative paths such that
they are found in the Q-1 step and give greater clarity of where the
file exists relative to the file including it. Using paths relative to
src/ via having src/ be an -I path, whether quoted or with <>, may be
slightly neater, but is somewhat confusing and unnecessarily
inefficient, especially in quoted form.
 - For plugins/modules, certain paths like modules/access/ are being
given as -I paths. Whether it is clear/sensible to do this when relying
upon shared files rather than quoted relative paths is difficult to
say, since I've not assessed it, but would be a lot of work to change.

This is what the patch set in its current form achieves.

> This also allows reusing standard header names in local projects
> without 
> wondering if it will be used or the system ones.

There is already no issue here. If the file is in the same folder, you
just use quotes. If it's in a different folder, then you should use a
relative path with quotes. You thus have certainty over what file is
used. If on the other hand you use <> or quotes with the file not being
in the same directory combined with an -I path, only then so you need
to worry about which one is going to be used, because it depends upon
the order in which -I paths are searched by the compiler, or whether
the compiler jumps straight to system paths for very common names.