-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
SF.12: Rewrite to eliminate the notions of "project" and "component"; also copyedit #1666
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19296,26 +19296,38 @@ The [standard](http://eel.is/c++draft/cpp.include) provides flexibility for comp | |
the two forms of `#include` selected using the angle (`<>`) or quoted (`""`) syntax. Vendors take | ||
advantage of this and use different search algorithms and methods for specifying the include path. | ||
|
||
Nevertheless, the guidance is to use the quoted form for including files that exist at a relative path to the file containing the `#include` statement (from within the same component or project) and to use the angle bracket form everywhere else, where possible. This encourages being clear about the locality of the file relative to files that include it, or scenarios where the different search algorithm is required. It makes it easy to understand at a glance whether a header is being included from a local relative file versus a standard library header or a header from the alternate search path (e.g. a header from another library or a common set of includes). | ||
Nevertheless, the guidance is to use the quoted form for including files that exist at a relative path | ||
to the file containing the `#include` statement and to use | ||
the angle bracket form everywhere else, where possible. This encourages being clear about the locality | ||
of the file relative to files that include it, or scenarios where the different search algorithm is | ||
required. It makes it easy to understand at a glance whether a header is being included from a locally | ||
relative file, versus a standard library header or a header from the header search path (e.g. a header | ||
from another library or from a common set of includes). | ||
|
||
##### Example | ||
|
||
// foo.cpp: | ||
#include <string> // From the standard library, requires the <> form | ||
#include <some_library/common.h> // A file that is not locally relative, included from another library; use the <> form | ||
#include "foo.h" // A file locally relative to foo.cpp in the same project, use the "" form | ||
#include "foo_utils/utils.h" // A file locally relative to foo.cpp in the same project, use the "" form | ||
#include <component_b/bar.h> // A file in the same project located via a search path, use the <> form | ||
#include <some_library/common.h> // A file that is not locally relative, included from another library: use the <> form | ||
#include "foo.h" // A file located relative to foo.cpp: use the "" form | ||
#include "foo_utils/utils.h" // A file located relative to foo.cpp: use the "" form | ||
#include <component_b/bar.h> // A file located via the header search path: use the <> form | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Users commonly believe that if an include is from the same library (or codebase or project or whatever you want to call it), then it must use the If there's one thing I'm trying to do with all of these comments, it's to fix this misconception. I fear that this PR pulls us back towards reinforcing that misconception. It's not quite clear enough that component_b is part of the same whatchamacallit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I struggled with this too; the whatchamacallit is, well... a project. which means we have to choose between defining what a project is (pro: being more crisp about how to include things from a project) or leaving it intentionally up to the user to define (pro: steering clear of defining what a project is in the guidelines, which is by nature a very liberal concept). I think I'm ok erring on the side of not defining what a project is here, and letting users figure out how best to include things from within one. The guideline is clear about how "" should be used, but if for some reason they're still adamant that "" must be used within their project, they'll have to align that with their policy about relatively accessing everything within their project (this should be intentionally left up to the user: some are ok with "..\bar\bar.h", some might want <foo/bar/bar.h>) As a funny anecdote, I'm actually trying to dispel the opposite myth in my codebase: a lot of developers are under the impression that <> must always be used from within a project (from what I can tell it's a myth created because so many have been bitten by differences in the "" vs <> they inevitably decide to stick with one) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That pretty much sums up my attitude, except: "A lot of cats are under the impression that stoves are not for sitting on (from what I can tell it's a myth created by once sitting on a hot stove)" 😉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "project" is mostly a Microsoft IDE-ism. We need to look beyond the particular workflow of a specific environment and implementation at one point of software organization. |
||
|
||
##### Note | ||
|
||
Failing to follow this results in difficult to diagnose errors due to picking up the wrong file by incorrectly specifying the scope when it is included. For example, in a typical case where the `#include ""` search algorithm might search for a file existing at a local relative path first, then using this form to refer to a file that is not locally relative could mean that if a file ever comes into existence at the local relative path (e.g. the including file is moved to a new location), it will now be found ahead of the previous include file and the set of includes will have been changed in an unexpected way. | ||
Failing to follow this results in difficult-to-diagnose errors due to picking up the wrong file by | ||
incorrectly specifying the scope when it is included. For example, in a typical case where the `#include ""` | ||
search algorithm might search for a file existing at a locally relative path first, then using this form to refer | ||
to a file that is not locally relative could mean that if a file by that name ever comes into existence at the | ||
locally relative path (e.g. the including file is moved to a new location), it will now be found ahead | ||
of the previous include file and the set of includes will have been changed in an unexpected way. | ||
|
||
Library creators should put their headers in a folder and have clients include those files using the relative path `#include <some_library/common.h>` | ||
Library creators should put their headers in a folder and have clients include those files using | ||
angle brackets: `#include <some_library/common.h>`. | ||
|
||
##### Enforcement | ||
|
||
A test should identify headers referenced via `""` could be referenced with `<>`. | ||
A tool could identify headers referenced via `""` that could have been referenced with `<>`. | ||
|
||
### <a name="Rs-namespace"></a>SF.20: Use `namespace`s to express logical structure | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Moving my reply inline, so that we can thread replies if needed)
Well, that's the tautological part. If the guideline intends to say merely "You should use
""
for headers you intend to find via the""
search path, and<>
for headers you intend to find via the<>
search path," then that's completely tautological. I think the useful guideline in this area would say something about which kinds of headers you should try to find via each of those search paths. E.g., I'm sure we agree "You should always intend to findiostream
via the<>
search path." IMHO it should also be acknowledged that the build system is likely to have vastly greater control over the<>
search path than over the""
search path, and so<>
should be preferred by default because it'll be easier for the human builder to configure.Btw, I agree that "locally relative" is a bit unclear and/or tautological, but consider that
#include <sys/types.h>
provides a pathname relative to somewhere. Essentially nobody ever uses#include
on an absolute path. So the (ill-defined) "local" part of the phrase is actually much more important to the meaning than the "relative" part. (Unfortunately when we try to pin it down we end up trying to define "project" or "codebase" again, which I want to push out of scope for this PR if at all humanly possible.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the intent is clear from the title, verbatim:
"" is for relative files (not "" files) and <> is for everything else (not <> files)
That's important because there can be non-relative files in "" (e.g. Apple's user-header-search-path) and there can be relative files in <> (e.g. codebases like the one you mention that exclusively use <>).
The guideline attempts to give examples of files that should be included using each type of search, and the phrase "a header from the header search path" is currently ambiguous; it is important to clarify for the given examples "another library or common includes" which header search path should be used (e.g. the <>-form)
is also local to somewhere too; "relative" and "local" are both terms that require context. The implied context is "to the including file", and this is explicit in the title (and at least a few other occurrences in the text). I think using "relative" from there on reasonably implies "to the including file", but if that's not clear it's also fine to explicitly say "relative to the including file" for every occurrence of "relative" (verbose, but if there's confusion and it dispels it, then it's worth it).
I found another usage of "alternate search path":
We should probably change any occurrence of "different" or "alternate" search path to a term we come up with for the <>-search that is configurable by the user. Or we don't want to term it, then another option is to rephrase it to just 2 distinctions "relative" and "non-relative" (but this also isn't specific to this PR, so it only needs to be addressed if we're also addressing usage of "alternate").