4 ms·
In my experience you should NEVER have const instance fields. Ever. If you want a const reference, just make a pointer to const. If you need a const value, just
by wfunction 10y ago
In my experience you should NEVER have const instance fields. Ever. If you want a const reference, just make a pointer to const. If you need a const value, just leave it mutable and access it through a const getter or work around it some other way. The language just doesn't seem to have been designed to make this particular feature actually worthwhile.
- keldaris 10y agoIt's very worthwhile for performance, and somewhat helps limit basic programmer errors. I can't imagine a single reason to tell people not to use const instance fields. On the other hand, telling people not to treat const as an absolute guarantee is obviously good advice and what you said about const& and getters is spot on.
- wfunction 10y agoYou're gonna need a citation (preferably, an example) for the performance claim. As for this comment of yours: > I can't imagine a single reason to tell people not to use const instance fields. That's weird, did you not read the blog post? It literally gave you a reason not to, you don't need to imagine anything. If you don't think it's a compelling reason then that's another story, but it's a valid reason.
- keldaris 10y agoIt's impossible to provide general benchmarks, since the impact will be completely case specific. However, the reason I didn't provide a citation is because the OP already gives an example - blink::serializedCharacterData gets moved over the read-only data segment if you add const, with obvious consequences for, say, thread-local accesses in tight loops. In my own code, I've seen adding a const modifier result in 20-60% perf improvement in fairly extreme cases. In other cases (probably most cases), it won't change much. More importantly, I've never seen an example where adding const would ever decrease CPU performance by any amount at all (other than compiler bugs, which can ruin any language feature). Accordingly, my personal recommendation is that everything that can be const should be. It's an easy thing to do that may help and definitely won't hurt. > That's weird, did you not read the blog post? It literally gave you a reason not to, you don't need to imagine anything. If you don't think it's a compelling reason then that's another story, but it's a valid reason. That's an outright compiler bug, not a general language reason. Obviously, it's a valid caveat if you're using that specific compiler and care about that edge case, but it's hardly a general argument.
- wfunction 10y ago> However, the reason I didn't provide a citation is because the OP already gives an example - blink::serializedCharacterData gets moved over the read-only data segment if you add const, with obvious consequences for, say, thread-local accesses in tight loops. Uh, that wasn't an instance field, was it? https://codereview.chromium.org/2608823002/diff/20001/third_party/WebKit/Source/platform/text/CharacterPropertyDataGenerator.cpp https://codereview.chromium.org/2608823002/diff/20001/third_... > That's an outright compiler bug, not a general language reason. It's not a bug, just a missed performance optimization. The code behaves correctly. And I was merely responding to your comment, "I can't imagine a single reason to tell people not to use const instance fields." The underlying language reason I have for it is not something you may agree with, but it's that it Seems Wrong (TM) to me for the fields of a class to dictate how the class's instances can be constructed or assigned to. Again, I don't claim I can convince you here. It just seems like poor design to me, and it's given me trouble so many times without once actually providing me a benefit. So it's a reason. YMMV.
- keldaris 10y ago> Uh, that wasn't an instance field, was it? https://codereview.chromium.org/2608823002/diff/20001/third_.. https://codereview.chromium.org/2608823002/diff/20001/third_.... Fair point, I was indeed being insufficiently precise. That code is a typical example of how const helps performance, concrete performance gains from applying const to instance fields in particular are much rarer in practice. > It's not a bug, just a missed performance optimization. The code behaves correctly. I think the MSVC devs classify it as a codegen bug since the cause is a logic error in the optimizer. The code does behave correctly, but it's reasonable to expect that optimization to happen, and it does in GCC / clang. My point here is simply that in this case MSVC is the outlier, therefore the example does not constitute a general argument against using const in this context. > The underlying language reason I have for it is not something you may agree with, but it's that it Seems Wrong (TM) to me for the fields of a class to dictate how the class's instances can be constructed or assigned to. I happen to have the opposite preference, namely that if you have constant data in a struct, it makes sense to say so at the declaration site for clarity. Regardless, it's a perfectly reasonable stylistic preference. I only argued against it because the phrase "NEVER have const instance fields. Ever." seems much too strong for what's ultimately a subjective choice. In doing so, however, I may have erred in the opposite direction and made overly strong statements myself. Mea culpa. > It just seems like poor design to me, and it's given me trouble so many times without once actually providing me a benefit. So it's a reason. YMMV. It's definitely a valid personal preference. When you say using const has given you trouble many times, do you have any particularly poignant example in mind?
- Arnt 10y agoYou can overload on const in C++, so int a(foo b) and int a(const foo b) can coexist. We used this in Qt 2.0, there were a few cases where we could get significantly more oomph if we knew the argument was const. Ff a function takes a struct (or class) as argument can calls otherfunction(foo.bar), then the constness of foo's bar field matters. The same might apply to fields of this.
- wfunction 10y agoNobody said it doesn't "matter" whether you put const. I was saying it's not worthwhile.
- pontobart 10y agoYou cannot overload on the const. If you try to define both int a(int b) and int a(const int b) the compiler will complain about a redefinition. You can declare the function using int a(int b) in the header and then define it via int a(const int b).
- revelation 10y agoI'm also with the VC team here, a const but not static instance field? Their stance seems correct that that should be initialized - in a constructor.
- wfunction 10y agoNot sure why you replied to my comment (seems like an unrelated comment?), but anyway, this should work too; you shouldn't need a constructor: struct { const int x; } y = { 1 };
- brucedawson 10y agoVC++ 2010 used to think that too and would generate warning C4610 for const non-static members. However that warning was wrong, and was changed to not fire in that case. See this link: https://connect.microsoft.com/VisualStudio/feedback/details/866320/incorrect-warning-c4610 https://connect.microsoft.com/VisualStudio/feedback/details/...
- wyldfire 10y agoWhat's the drawback for a const member? IMO `const` should be used liberally: members, locals and other declarations. When you're on a team and you have a data structure that has const members, it should give that newer/unfamiliar team member pause when they get the compilation error because they added a method that modifies the `const` member. Now they have an opportunity to take a step back and ask "Oh, why was it designed this way? What other assumptions in the code are invalidated if I need to remove the `const`?" `const` for locals is good for those notorious ten-line functions that over time multiply into a 150-line function. Yes, we should refactor that mega function into something easier to digest. But right now I am trying to figure out this bug: I can see that ^^ up there 'foo' was initialized as a particular value, and down here it looks like it must have changed. Instead of looking through all of the hundred lines of code to see who might've updated 'foo', I know right away that a likely explanation is stack corruption. This is BTW why I love that rust has opt-in mutability. It feels almost a little silly sprinking `const` thither and yon when it's only a few select places that it's actually un-needed.
- brucedawson 10y agoAuthor here. I'm a big believer in adding const everywhere - local variables, globals, member functions, etc. Marking globals as const can move them to the read-only segment which has some modest performance improvements, but in most other cases const doesn't affect code-gen at all. Using const prevents programmers from doing certain things, and the compiler enforces that. That suggests that the compiler already knows whether or not programmers are doing those things - it doesn't need a 'const' hint to tell it. And in those cases where it can't tell, the 'const' hint is usually not sufficient for it to depend on. In short, with the exception of moving globals to the read-only segment I am not aware of any performance improvements from using const.
- wfunction 10y ago> What's the drawback for a const member? Check out this subthread: https://news.ycombinator.com/item?id=13356101 https://news.ycombinator.com/item?id=13356101
- phaedrus 10y agoI think you're getting downvoted because you asserted advice as an absolute and gave no explanation why/when it applies. I happen to agree with you empirically, but I would phrase it thus: const member variables can be problematic in C++ because it disables some of the compiler-generated functions such as memberwise assignment. It violates the law of least astonishment because it seems natural to use const for the members of a small "value type" (e.g. Point, Complex, etc.) and it seems natural to store such objects in std container (e.g. std::vector), but adding the "const" on the member fields makes the object incompatible with the std containers.
- wfunction 10y agoYeah, you're saying more or less what I was thinking here. https://news.ycombinator.com/item?id=13356101 https://news.ycombinator.com/item?id=13356101 Indeed I should've written it in the original comment.