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

add F.12: Describe when to use the different forms of #include #1456

Closed
wants to merge 7 commits into from
Closed

add F.12: Describe when to use the different forms of #include #1456

wants to merge 7 commits into from

Conversation

ChrisGuzak
Copy link
Contributor

No description provided.

@jwakely
Copy link
Contributor

jwakely commented Jun 26, 2019

What is "INCLUDES"?

@jwakely
Copy link
Contributor

jwakely commented Jun 26, 2019

The spellchecker doesn't understand that HTML anchors aren't readable text, so the word "incscope" needs to be added to scripts/hunspell/isocpp.dic to fix the failing Travis CI test.

@ChrisGuzak
Copy link
Contributor Author

What is "INCLUDES"?

See my update. for MSVC how these are specified is described here.

@jwakely
Copy link
Contributor

jwakely commented Jun 27, 2019

I assumed it must be a MSVC thing, but the docs say INCLUDE not INCLUDES. And other compilers don't use anything by either of those names.

Is this really something that should be enforced by the checker?

CppCoreGuidelines.md Outdated Show resolved Hide resolved
CppCoreGuidelines.md Outdated Show resolved Hide resolved
CppCoreGuidelines.md Outdated Show resolved Hide resolved
@ChrisGuzak
Copy link
Contributor Author

ChrisGuzak commented Jun 28, 2019

Is this really something that should be enforced by the checker?

no. I've generalized this.

@hsutter
Copy link
Contributor

hsutter commented Aug 1, 2019

Editors' call: Several of us are personally sympathetic to using <> for standard headers and "" for other headers, which is implied by the standard (http://eel.is/c++draft/cpp.include) but not sure this warrants a rule. Does this have a reasonable chance of not being ignored if we try to mandate it?

(We are looking forward to a future guidelines to not use #include at all, in a modules world, someday.)

@hsutter hsutter self-assigned this Aug 1, 2019
@cubbimew
Copy link
Member

cubbimew commented Aug 1, 2019

FWIW, cppreference suggests "intent" and "typical implementation" for both angle bracketed and quoted includes, while the standard just says "implementation-defined" for both.

@hsutter hsutter assigned GabrielDosReis and unassigned hsutter Aug 8, 2019
@hsutter
Copy link
Contributor

hsutter commented Aug 8, 2019

Editors call: We know we should use <> for standard headers. We know we should use "" for our own project's files. We don't have a rule for the "in between" that we think people can follow universally and welcome new suggestions in this thread.

@ChrisGuzak
Copy link
Contributor Author

Any progress on this? I'd really like something to point to when people write
#include "string"

@jwakely
Copy link
Contributor

jwakely commented Aug 14, 2019

The standard already seems pretty clear that #include "string" is dumb wrong. It says angle brackets searches for "a header" which in standardese means one of the standard library headers. It says double quotes searches for "a source file" which in standardese means some other header file, not a standard library header. Since <string> is certainly "a header" (i.e. a standard library header) it should be included with angle brackets.

There's also a note that says "in general programmers should use the < > form for headers provided with the implementation, and the " " form for sources outside the control of the implementation."

@ChrisGuzak
Copy link
Contributor Author

thanks... I was not aware of that distinction. I suspect many others (most?) are not either. I guess a pointer to the standard would be a second choice to having this guideline added.

@jwakely
Copy link
Contributor

jwakely commented Aug 15, 2019

The remaining question is what is "a header" and what isn't "a header", but that's why the non-normative note suggests distinguishing headers/files that are "provided by the implementation" from other ones. But in any case, there's no question about <string>.

@hsutter
Copy link
Contributor

hsutter commented Aug 15, 2019

Editors call: Thanks! @ChrisGuzak could you please update the PR to:

  • say we should use <> always, and only, for standard headers, and "" for all non-standard headers
  • link to the standard paragraph

and we'd be glad to merge it.

@ChrisGuzak
Copy link
Contributor Author

been a while but I updated the PR.

@ChrisGuzak ChrisGuzak changed the title add SF.12 Use the quoted or angle bracket form of #include to identify the scope of dependencies add F.12: Use the quoted or angle bracket form of #include to distinguish standard headers from others Nov 22, 2019

##### Note

This is specified [here](http://eel.is/c++draft/cpp.include) in the standard.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like http://eel.is/c++draft/cpp.include#8 would be a slight improvement on the link

@johnmcfarlane
Copy link
Contributor

I'm opposed to this advice.

On both POSIX systems and under MSVC (with minor caveats) the other major difference between "" and <> forms is that "" first searches for the header relative to the including file. Hence, using "" to include a file which is not a system file but which is intended to be found via a search path (e.g. on specified with -I, -isystem or /I) has correctness and efficiency problems:

  • The wrong file might be included by mistake or
  • a nonsensical path will be searched first.

I think that the correct advice should be to use <> for headers/files which are intended to be found via a search path and for "" to be used to located files which are located physically relative to the including file.

I've written more about this in a S.O. answer here.

@ChrisGuzak
Copy link
Contributor Author

@johnmcfarlane that was my position coming into this but I took the turn that Herb suggested.

@MikeGitb
Copy link

Sorry, but that's ridiculous. In addition to what @johnmcfarlane said, standard library headers are already easy identifiable, because a) most c++ probably know them by heart and b) they are virtually the only headers without a file extension.

Does this have a reasonable chance of not being ignored if we try to mandate it?

Virtually every code base I've worked on ignores this and follows the rule suggested by John. I really don't see what this is supposed to accomplish.

##### Reason

To understand dependencies it is important to know where included files come from. Given the different search algorithms,
the angle form (`<>`) identifies standard headers, the quoted (`""`) everything else.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This contradicts the note I mentioned:

in general programmers should use the < > form for headers provided with the implementation, and the " " form for sources outside the control of the implementation.

That strongly suggests that angle brackets should be used for non-standard headers "provided with the implementation" such as Windows.h and unistd.h, or system libraries like zlib.h or Python.h. Limiting it to just standard headers is too restrictive, and ignores the "in between" cases that are not standard headers but also not one of the project's own source files.

I have no idea how Windows does things, but on many POSIX systems it doesn't make sense to do #include "X11/Xlib.h" with quotes but #include <stdio.h> with angle brackets, when both are likely to be found in /usr/include.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still too restrictive. It's often appropriate to use <> to include headers from within the same project.

@jwakely
Copy link
Contributor

jwakely commented Dec 12, 2019

I'm not convinced the guidelines should be trying to codify this at all.

If the original motivation is just to be able to correct nonsense like #include "string" then point to the standard, or just use a stick to drive those people off a cliff into the sea.

@shaneasd
Copy link
Contributor

@jwakely, I don't agree with the recommendation being made in this pull request but I think it does make sense to codify something.

I think the distinction between <> and "" is far from obvious particularly given how much it does (or is allowed to?) vary between implementations. I suspect that in many cases the distinction is largely moot and it's more about communicating intent through the choice made and it's useful for the guidelines to give a definite meaning that is communicated by each choice. There are certainly some things where it seems like a programmer would have to go out of their way to shoot themselves in the foot but I don't think this is such a case.

The target readership is "All C++ programmers" and I would be especially sympathetic to programmers coming from other languages where there aren't two near identical ways of importing referenced code.

I'd also like to think that a c++ programmer could do well referencing only the core guidelines and other online material without having to read the standard. Perhaps I'm mistaken in my understanding of the purpose of the document but I didn't think the average c++ programmer was expected to read it. As someone who hasn't read most of it, I am of the understanding that it provides instruction on how to correctly implement a compiler/stl, not an arbitrary c++ program. I think there are lots of ways in which the c++ standard allows you to write bad c++ code and the point of the core guidelines is to tell you how not to.

It also seems given your and @ChrisGuzak 's contradictory interpretations of the standard that it is not sufficiently explicit to provide unambiguous guidance.

It's possible that your position is simply that regardless of whether it is obvious or not, the impact of getting it wrong is low, to which I offer no counterargument. I haven't considered the demands on the editors time or the negative impact of overly numerous or overly strict rules.

@jwakely
Copy link
Contributor

jwakely commented Dec 12, 2019

I haven't considered the demands on the editors time or the negative impact of overly numerous or overly strict rules.

I certainly consider that every time there's a proposal for a new rule. Many suggestions look reasonable in isolation, but adding every one of them to the guidelines makes them bloated and less useful.

@jwakely
Copy link
Contributor

jwakely commented Dec 12, 2019

... and if the subject is divisive or not universally agreed upon, pretending there's only one right way to do it can actually have negative value. The current proposed wording ("the angle form (<>) identifies standard headers, the quoted ("") everything else") falls into that category for me.

It might be better to say nothing than to give bad advice.

@johnmcfarlane
Copy link
Contributor

tl;dr +1 @jwakely, let's maybe back away from this

@johnmcfarlane that was my position coming into this but I took the turn that Herb suggested.

I'm not entirely sure about that. I find it tricky to explain my reservation about the advice: "Use the quoted or angle bracket form of #include to distinguish standard headers from others".

If a header or source file from a project includes another header from the same project, but that included header is found via a header search path (as opposed to being relative to the including file), then angle brackets are appropriate. I believe that's contrary to the title of this PR at this time.

I don't believe that this is opinion or style. This is my interpretation of docs such as these (somewhat ambiguous) ones for GCC and this MSVC page. What they document is valuable functionality. However, they do somewhat contradict this note in the standard which @hsutter points to. (I think the standard maybe takes a wrong turn here!)

I think it's ill advised to use "" to include a file that is found via a search path (e.g. one provided by -I). It's inefficient and will occasionally result in the wrong file being included.

For example, consider a user's project in the /my_project/ directory. Say it includes a copy of GSL at /my_project/external/, and uses the flag, -I/my_project/external/gsl/include (GCC/Clang) or /I/my_project/external/gsl/include (MSVC).

Now consider a file, /my_project/src/my_source.cpp which observes F.12:

#include "gsl/gsl_assert"

A typical compiler will first look for the file, /my_project/src/gsl/gsl_assert. That's the wrong location and a waste of time. If (by freak accident) there's a file in that location, you included the wrong file in a way which was rather surprising, a pain to diagnose and entirely avoidable.

If, however, your file looks like this:

#include <gsl/gsl_assert>

Then everything works as intended but F.12 is violated.

Given the hot air required to explain this, I sympathize with @jwakely's inclination to slowly walk away from this topic. However, I'm also happy to help draft an amended rule (and/or the note) which plays nice with common toolchains. Perhaps:

F.12 Prefer the angle bracket form of #include where you can and the quoted form everywhere else

@alex4747-pub
Copy link

I would prefer always use '<...>', so if we have to substitute a .h file for unit test compilation we can do it for each and every file.

@johnmcfarlane
Copy link
Contributor

@alex4747-pub you can achieve this without contravening my suggested advice if you add every location to your header search path. I'm not sure it's necessary for situations where foo.cpp includes foo.h located in the same source file. But it's no need to prevent other users from using "" correctly and safely.

@ChrisGuzak
Copy link
Contributor Author

I'd be happy to go with

F.12 Prefer the angle bracket form of #include where you can and the quoted form everywhere else

if others agree

@ChrisGuzak
Copy link
Contributor Author

ChrisGuzak commented Feb 22, 2020

#1570. This works well for me, avoids the 3rd rails and matches my (naive) take on this going in.

@ChrisGuzak ChrisGuzak changed the title add F.12: Use the quoted or angle bracket form of #include to distinguish standard headers from others add F.12: on when to use the different forms of #include Feb 23, 2020
@ChrisGuzak ChrisGuzak changed the title add F.12: on when to use the different forms of #include add F.12: Describe when to use the different forms of #include Feb 23, 2020
@ChrisGuzak
Copy link
Contributor Author

#1570 replaces this, so I'll close this one.

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

Successfully merging this pull request may close these issues.

None yet

10 participants