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

Headers module #74

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Headers module #74

wants to merge 10 commits into from

Conversation

rwdougla
Copy link
Contributor

@rwdougla rwdougla commented Feb 23, 2022

resolves #3


#### Background/Required Knowledge

* All of the above.

Choose a reason for hiding this comment

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

Is "All of the above" on line 77 referring to all of the foundational background? I think that it would be clearer to say this explicitly. Incidentally, is it implied that all background for a topic at the foundational level is also required for the main level? If so, then perhaps such a statement is unnecessary?

Would like to get cross-references in advance section to other modules which are coupled to headers, such as classes, templates, modules, etc.
@vulder vulder linked an issue Apr 29, 2022 that may be closed by this pull request
@vulder
Copy link
Collaborator

vulder commented Jul 8, 2022

/ok-to-test

feeling of small-group was to omit advanced section, as each advanced topic seemed more relevant to a different module
@rwdougla rwdougla marked this pull request as ready for review January 17, 2023 16:12
@rwdougla rwdougla changed the title WIP: Straw-man "Headers" for small-group session Straw-man "Headers" for small-group session Jan 17, 2023
@rwdougla rwdougla changed the title Straw-man "Headers" for small-group session Headers module Jan 17, 2023
* Usage of standard library code requires inclusion of various header files
* Give examples of various header files and when they may be needed for inclusion
* Include links to reference materials for finding out what headers are used for various feature sets
* When using standard library facilities, can either use `using std::vector; vector<int> v;`, or `std::vector<int> v;`, or `using namespace std; vector<int> v;`, though modern practice strongly discourages invoking `using namespace std;`
Copy link
Contributor

Choose a reason for hiding this comment

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

That is a nice example but that does not relates to headers. I think it relates to templates and I am not sure if we should mention it there.

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

From what I see, this relates directly to headers. The reasoning this point tries to get across is the fact that using namespace x, in a header introduces all names within x into all files that include the specific header (even transitively). This not only makes it likely to have name clashes but also increases the chance that, for example, some usages of functions with the same name as is present in x, select a different (better fitting) overload, which can introduce subtle errors.

I assume, @rwdougla wanted to have this point covered when talking about headers, so that students are aware and do not misuse using namespace x.

Maybe we should highlight the reasoning here so it's clearer.

Copy link
Collaborator

@vulder vulder left a comment

Choose a reason for hiding this comment

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

Nice, work.

@rwdougla What's the status of this topic, should we table it for a SG20 discussion? It seems quite ready.

sources/modules/compilation-model/headers.md Outdated Show resolved Hide resolved
sources/modules/compilation-model/headers.md Outdated Show resolved Hide resolved
sources/modules/compilation-model/headers.md Outdated Show resolved Hide resolved
sources/modules/compilation-model/headers.md Outdated Show resolved Hide resolved
sources/modules/compilation-model/headers.md Outdated Show resolved Hide resolved
sources/modules/compilation-model/headers.md Show resolved Hide resolved
@diehlpk
Copy link
Contributor

diehlpk commented Jan 8, 2025

@rwdougla could you please have a look and accept the changes?

@vulder
Copy link
Collaborator

vulder commented Jan 22, 2025

@diehlpk I talked to @rwdougla. I'll take care of the PR on the weekend and clean in up :)

@diehlpk
Copy link
Contributor

diehlpk commented Jan 22, 2025

@vulder Perfect. Thanks. We will work next week on the lambda pull request.

@vulder vulder requested review from diehlpk and vulder January 26, 2025 13:28
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.

[TOPIC] Headers
5 participants