Change how includes for input file directory works
AcceptedPublic

Authored by Phyx on Oct 8 2017, 8:26 AM.

Details

Reviewers
bgamari
hvr
austin
Trac Issues
#14312
Summary

GHC Used to only allow for one include mode, namely -I.
The problem with -I includes is that it supercedes all other
includes, including the system include paths.

This is not a problem for paths requested by the user, but it is a
problem for the ones we implicitly derive and add.

In particular we add the source directory of the input file to the
include path. This is problematic because it causes any file with the
name of a system include, to inadvertently loop as the wrong file gets
included.

Since this is an implicitly include, and as far as I can tell, only done
so local includes are found (as the sources given to GCC reside in a temp
folder) then switch from -I to -iquote.

Test Plan

./validate

Phyx created this revision.Oct 8 2017, 8:26 AM
Phyx added a comment.Oct 8 2017, 8:29 AM

Accompanying haddock pull request is here https://github.com/haskell/haddock/pull/689

bgamari requested changes to this revision.Oct 9 2017, 2:14 PM

Yes in principle but I have a few concerns.

compiler/main/DynFlags.hs
686

Doesn't the order matter here? In the singleton case we prepend whereas here we append.

688

Can you add comments to this and the above function clarifying what they do?

This revision now requires changes to proceed.Oct 9 2017, 2:14 PM
austin resigned from this revision.Nov 9 2017, 5:41 PM
Phyx updated this revision to Diff 14959.Dec 20 2017, 3:52 PM
Phyx marked 2 inline comments as done.
  • Fix include
Phyx added a comment.Dec 20 2017, 3:58 PM

Having done the archeology on this, it seems the original support for CApi did not add any tests 36f8cabecd5a8320ee174abb56e73841a5cbc9c7 later an unrelated commit for hs-boot 06d46b1e4507e09eb2a7a04998a92610c8dc6277 added the currently directory as a general include directory.

So when these CApi tests were added they just happen to work.

Changing the include style from <> to "" is according to the Haskell report:

Specification of Header Files A C header specified in an import declaration is always included by #include "chname". There is no explicit support for #include <chname> style inclusion. The ISO C99 standard guarantees that any search path that would be used for a #include <chname> is also used for #include "chname" and it is guaranteed that these paths are searched after all paths that are unique to #include "chname". Furthermore, we require that chname ends in .h to make parsing of the specification of external entities unambiguous.

and the C semantics safe:

#include "file"

    This variant is used for header files of your own program. It searches for a file named file first in the directory containing the current file, then in the quote directories and then the same directories used for <file>. You can prepend directories to the list of quote directories with the -iquote option.

So this should be more correct and not unexpectedly break stuff

Phyx added a reviewer: hvr.Dec 20 2017, 3:59 PM
Phyx added inline comments.Dec 22 2017, 2:37 PM
docs/users_guide/8.4.1-notes.rst
159 ↗(On Diff #14959)

This needs to be in the 8.6 release notes, but that doesn't exist yet

Phyx updated this revision to Diff 14972.Dec 23 2017, 3:26 AM
  • Rebases
bgamari accepted this revision.Sun, Jan 21, 9:33 PM

Thanks for the ping, @Phyx!

Looks good to me modulo one documentation wibble. I can fix this when I merge.

docs/users_guide/8.6.1-notes.rst
30
:ghc-flag:`-iquote`
This revision is now accepted and ready to land.Sun, Jan 21, 9:33 PM