-
Notifications
You must be signed in to change notification settings - Fork 967
Update destructors-cpp.md #4846
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
Conversation
Demo in example is almost completely wrong. There are some inconsistencies among separate parts of program code: 1) The parameter of constructor has to be const qualifier. "string" is also called as "character constants", "string constant" and "anonymous string" in C++. This is because a "string" in the C++ standard has the "const qualifier". But the situation in C is slightly different. Since the "const keyword" did not exist in C until the "ANSI C 1989" standard, "string literals" don't have a "const qualifier" for such historical reasons. Hence "non-const string" in C is evaluated as "undefined behavior". Because of the "string literal" that sent as argument to constructor in creation of class object is located in memory in the ".rodata section" of the "data segment", but the constructor parameter "pointer" (char *ch) to string even though itself is in the ".rwdata section" of data segment, it "points" to the "first character address", i.e. "base address" of "string" located in the "read-only section" and store this address. That is why must be added the "const qualifier" that indicating it's "pointing" to "const character". So the compiler generates an error - E0289. To resolve this added "const qualifiers" in 2 places. 2) Class members in private section were initialized with "nullptr" and "zero" respectively. 3) Since "delete [] _text" deallocates "*_text", but the "_text" pointer continues to point to the memory location (may store several garbage values) which has been released which then should not be used as it evaluated as undefined behaviour. To prevent this issue added necessarry line "_text = nullptr;" inside the destructor. For resolving there is also could be used a smart pointer, but I did not change the POD style. 4) In that case in which "destructors", swap functions, move operations, and default constructors should never throw, plus from the performance optimization perspective added "noexcept specifiers" for destructor in 2 places that allows the compiler to skip the process of exception handling which leads to faster execution of the program.
@shalala66 : Thanks for your contribution! The author(s) have been notified to review your proposed change. |
Learn Build status updates of commit b1ea2fb: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Can you review the proposed changes? When the changes are ready for publication, add a #label:"aq-pr-triaged" |
Remove some things that aren't strictly necessary
Learn Build status updates of commit 5e29c1a: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Learn Build status updates of commit 5fd0061: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Hi @shalala66, Thank you for taking the time to contribute to the docs! Really appreciate it. For some context: in general, these examples are not meant to be production-ready code--though they should compile (so thanks for fixing that). We assume people know how to program, so what we want to do in these examples is demonstrate some small point about the thing being documented in a way that can be understood quickly. We try not to obscure the point with all the normal error handling code people should know to write. For example, this code doesn't check to see if destructors are noexcept by default except in a couple situations that don't apply here. So, it's superfluous to add noexcept. You can test that this destructor is noexcept with something like static_assert(noexcept(str.~str()), "destructor isn't noexcept!"); For this example, keeping the size of the text as a member of the class doesn't add any value and just adds an element of complexity. So, I removed it. Setting _text to NULL is good practice, but not necessary because it gets set to NULL if the memory allocation fails. Setting it to NULL in the destructor is unnecessary because the instance is dead at this point. I'm not arguing that it isn't a good practice to do these things, but that it is unnecessary for the example. We are trying to just point out just the essentials here. Your changes + mine will be live by tomorrow. #sign-off |
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.
approved with changes
No, it should not compile and it's as soon as obvious in first looking at the code even without compile it.
It's not correct assumption from your side. Where is the defined that this place is only for experienced ones? Cuz, this platform where people learn necessary things from. That is why it should not be depend on knowledge of people. If someone appeals to this platform in that case he need to get information from, including demo code snippets in a correct way. You must not consider that only the people with enough knowledge make a visit to this platform. I could even say vice-versa.
Very good & valuable practice that I can't explain the importance of adding in microsoft/wil community.
I completely don't agree with you and It's most necessary in good practice. I can give you some examples exactly related in this part of your code and continue the dialog in this theme. Then this pointer would be a dangling pointer that points to invalid data or to data which is not valid anymore. |
The perfect-programmer presumption is going to be the death of us all. We're talking about learn.microsoft.com, yes? In reality, people learn from Internet examples. The over-simplification of code, lacking defensive programming methods, is going to instruct beginners on their way to becoming paid programmers with very bad habits. The "should know to write" will not happen if there are not exemplary examples in the work seen of others. I have no magic formula for how to keep error-detection from obscuring the point of an example. It is something worth exploring some sort of practices, like keys to notation/style, that evoke occasions of safe/defensive practices without obscuring the purpose of an example. The C/C++ language histories and design are not so helpful. We can't expect exceptions to be thrown as a way of saving us from writing safe code. I'll get off my soapbox now. I just want to pay my respect to the concerns of @shalala66 |
@orcmid , thank You for your attention and for sharing your opinion.
yes. |
@shalala66 , you totally misunderstood me: "No, it should not compile and it's as soon as obvious in first looking at the code even without compile" My point was that the code samples should compile, and the fact that this one didn't was a problem I was thanking you for fixing... Also, it's not my code. It's one of over 10,000 topics, many of which predate me by a very long time, that I have to somehow stay on top of, so there's a finite amount of attention I can give to each one. |
@TylerMSFT, all clear. Thank you. |
Guys, you provide reasonable feedback. I'll certainly think more about what to do here. Many of the reference articles were written a long time ago and have been through more than one publishing system and more than one philosophical iteration about how to approach them. Yes, the learn in learn.microsoft.com is about learning. But you've probably noticed that we have different categories of docs, and they have different purposes. Reference docs are generally about describing APIs. We aren't really trying to teach you how to program with those--we are trying to describe a system as succinctly and accurately as possible. By analogy, they are sorta like the dictionary in the sense that they provide the definition of the 'words' (APIs), but they won't tell you how to talk to your mother :-) We have how-to and tutorial content where the intent is to teach best practices and basics and so on. However, I do think the point about learning by example and the fear of cut-n-paste programming is important. I'll mull it over during the holiday. I do appreciate that you care about the docs! Obviously, we want to make them as good as possible. |
Oh, regarding the pull request being merged but you don't see it on the live site - that usually takes at least a day. Look for it tomorrow in the afternoon. |
Demo in example is almost completely wrong. There are some inconsistencies among separate parts of program code:
The parameter of constructor has to be const qualifier. "string" is also called as "character constants", "string constant" and "anonymous string" in C++. This is because a "string" in the C++ standard has the "const qualifier". But the situation in C is slightly different. Since the "const keyword" did not exist in C until the "ANSI C 1989" standard, "string literals" don't have a "const qualifier" for such historical reasons. Hence "non-const string" in C is evaluated as "undefined behavior". Because of the "string literal" that sent as argument to constructor in creation of class object is located in memory in the ".rodata section" of the "data segment", but the constructor parameter "pointer" (char *ch) to string even though itself is in the ".rwdata section" of data segment, it "points" to the "first character address", i.e. "base address" of "string" located in the "read-only section" and store this address. That is why must be added the "const qualifier" that indicating it's "pointing" to "const character". So the compiler generates an error - E0289. To resolve this added "const qualifiers" in 2 places.
Class members in private section were initialized with "nullptr" and "zero" respectively.
Since "delete [] _text" deallocates "*_text", but the "_text" pointer continues to point to the memory location (may store several garbage values) which has been released which then should not be used as it evaluated as undefined behaviour. To prevent this issue added necessarry line "_text = nullptr;" inside the destructor. For resolving there is also could be used a smart pointer, but I did not change the POD style.
In that case in which "destructors", swap functions, move operations, and default constructors should never throw, plus from the performance optimization perspective added "noexcept specifiers" for destructor in 2 places that allows the compiler to skip the process of exception handling which leads to faster execution of the program.