Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of angle brackets around file names for include statements #10

Closed
elfring opened this issue Aug 1, 2015 · 11 comments
Closed

Use of angle brackets around file names for include statements #10

elfring opened this issue Aug 1, 2015 · 11 comments
Assignees

Comments

@elfring
Copy link

elfring commented Aug 1, 2015

Would you like to replace any double quotes by angle brackets around file names for include statements?

@d-frey
Copy link
Member

d-frey commented Aug 1, 2015

We explicitly chose to use double quotes for internal includes, i.e., includes of one PEGTL header file from within another PEGTL header. That way we are independent of additional include paths and their order, thus more robust in some use cases.

I am not aware of any problem or drawback caused by this. Do you have any problem with it?

@d-frey d-frey self-assigned this Aug 1, 2015
@d-frey d-frey added the question label Aug 1, 2015
@elfring
Copy link
Author

elfring commented Aug 1, 2015

I suggest to reconsider the consequences of the following wording from the section "16.2 Source file inclusion" in the standard specification for the programming language "C++".

…
The named source file is searched for in an implementation-defined manner. If this search is not supported, or if the search fails, the directive is reprocessed as if it read

#include <h-char-sequence> new-line
…

@d-frey
Copy link
Member

d-frey commented Aug 1, 2015

You are quoting 16.2/p3, but that kind of wording is also found in 16.2/p2 which therefore also applies to #include directives using angle brackets:

16.2 [cpp.include]

2 A preprocessing directive of the form

# include < h-char-sequence > new-line

searches a sequence of implementation-defined places for a header identified uniquely by the specified sequence between the < and > delimiters, and causes the replacement of that directive by the entire contents of the header. How the places are specified or the header identified is implementation-defined.

(taken from N4140, emphasis mine)

That said, it is defined quite well by all implementations I know of (and which are relevant to the PEGTL's minimum requirements). The "duplicate search" can not happen as the file referenced is our own and thus the first candidate.

The last link you mentioned (from GCC) explicitly says:

#include "file"
This variant is used for header files of your own program. [...]

And that is exactly what we are doing.

@elfring
Copy link
Author

elfring commented Aug 1, 2015

There are different opinions about the handling of the involved implementation-defined behaviour.

  • Will header files be also searched outside the specified include directories if double quotes are used for the discussed preprocessor statement?
  • Is there a speed difference measurable if a file is not found there and the search will be retried with "the angle brackets inclusion method"?

@d-frey
Copy link
Member

d-frey commented Aug 1, 2015

Those points do not apply as the header we are referencing (in a relative way) is always found.

Also, you would break the use-case where a user copies pegtl.hh and the pegtl directory to his own project and doesn't even add the include path for pegtl. The user will only use

#include <my_project/my_parser_component/pegtl.hh>

And the PEGTL header would not even be able to use #include <pegtl/input.hh> as the user will not add -Imy_project/my_parser_component to the compiler flags.

This is an important and from our side supported use-case which can only be achieved with double quotes. Your suggestion would break that use-case. It would not be any faster (as the files we are including with double quotes are always found) and there is no actual problem. I'm thus closing this issue.

@d-frey d-frey closed this as completed Aug 1, 2015
@d-frey d-frey added the wontfix label Aug 1, 2015
@elfring
Copy link
Author

elfring commented Aug 1, 2015

… as the user will not add -Imy_project/my_parser_component to the compiler flags.

I suggest to think again about consequences from such an expectation.

I would generally prefer to exclude the potential for the inclusion of header files from unexpected directories. Can such a security detail become a bit more important occasionally?

@d-frey
Copy link
Member

d-frey commented Aug 1, 2015

Using double quotes is explicitly meant to exclude the "potential for the inclusion of header files from unexpected directories", thus preventing (security) issues. And we do not require the user to not include the path, it is just an option the user can chose if he/she wants to.

@elfring
Copy link
Author

elfring commented Aug 1, 2015

Would you like to support the use case that your header files should occasionally only be found from the installed system directories?

@ColinH
Copy link
Member

ColinH commented Aug 1, 2015

Yes, and it is currently supported by copying the PEGTL to a system include directory. We very purposefully and intentionally use double-quote includes inside the PEGTL.

@elfring
Copy link
Author

elfring commented Aug 1, 2015

Your library can also be installed as it is usual with other software.

  • Can any conflicts and version mismatches happen if the inclusion style will trigger the reuse of header files from other storage locations?
  • When do you want to choose between inclusion from "blessed" directories instead of a random and "convenient" folder?

@ColinH
Copy link
Member

ColinH commented Aug 1, 2015

No, there is no potential for conflicts, and no, we never include from a "random or convenient" folder unless it is the folder in which the PEGTL itself resides and was chosen by the user, in which case it is by definition no longer "random". With quotes the search for include files starts relative to the file containing the include, so there are no problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants