A colleague of mine was trying to debug some code that they wrote. They wanted to create an object on demand, but their function to return the on-demand object kept creating new instances rather than reusing the previously-created one.<\/p>\n
Here’s a simplified version.<\/p>\n
\/\/ For expository simplicity, assume single-threaded objects\r\n\r\nstruct Widget\r\n{\r\n Widget(bool debugMode = false);\r\n\r\n int m_count = 0;\r\n void Increment() { ++m_count; }\r\n};\r\n\r\nstruct Doodad\r\n{\r\n std::unique_ptr<Widget> m_widget; \/\/ created on demand\r\n\r\n Widget GetWidget()\r\n {\r\n if (!m_widget) {\r\n m_widget = std::make_unique<Widget>();\r\n }\r\n return m_widget.get();\r\n }\r\n};\r\n\r\nvoid Sample()\r\n{\r\n Doodad doodad;\r\n\r\n \/\/ Demand-create the widget and increment its counter.\r\n doodad.GetWidget().Increment();\r\n\r\n \/\/ This assertion fires?\r\n ASSERT(doodad.GetWidget().m_count > 0);\r\n}\r\n<\/pre>\nIt’s as if the GetWidget()<\/code> function is creating brand new empty widgets instead of creating one on its first call and reusing it for subsequent calls.<\/p>\nMaybe you can spot the problem.<\/p>\n
What struck me was that the return type of GetWidget()<\/code> was wrong. The m_widget.get()<\/code> returns a Widget*<\/code>, but the function returns a Widget<\/code> object. But how did this code even compile? The return type is wrong!<\/p>\nThe answer is a backward compatibility feature of C++ combined with a poor choice of default in the language design.<\/p>\n
The backward compatibility feature is that pointers can implicitly convert to bool<\/code>: The built-in boolean conversion is a comparison against nullptr<\/code>. This is a backward compatibility feature carried over from C.<\/p>\nThe poor choice of default in the language is that any constructor that can be called with a single parameter (though possibly with the help of some defaulted parameters) is by default usable as an implicit conversion. To opt out, you must use the explicit<\/code> keyword.<\/p>\nstruct Widget\r\n{\r\n \/\/ \u2193 usable as implicit conversion from bool\r\n Widget(bool debugMode = false)<\/span>;\r\n\r\n int m_count = 0;\r\n void Increment() { ++m_count; }\r\n};\r\n<\/pre>\nIf we write out the implicit conversion, the GetWidget<\/code> becomes<\/p>\n Widget GetWidget()\r\n {\r\n if (!m_widget) {\r\n m_widget = std::make_unique<Widget>();\r\n }\r\n return Widget(m_widget.get() != nullptr)<\/span>;\r\n }\r\n<\/pre>\nNow we see more clearly what’s going on.<\/p>\n
The pointer produced by m_widget.get()<\/code> is converted to a bool<\/code> by checking whether it is null. (And since the pointer inside m_widget<\/code> is always non-null by the time we get to the return<\/code> statement, the result is always true<\/code>.) And then we use the Widget<\/code> constructor that takes a single bool<\/code> parameter and use it as a conversion.<\/p>\nThe result is that the GetWidget()<\/code> method always returns a freshly-created Widget<\/code> in debug mode.<\/p>\nI was initially baffled as to how the original code compiled, seeing as the return type was wrong. I figured out that it was the implicit conversion by looking at the code generation. The code for the return<\/code> statement looked like this:<\/p>\n cmp QWORD PTR [rbx], 0 ; Q: m_widget.get() == nullptr?\r\n setne dl ; dl = 1 if non-null (constructor parameter)\r\n mov rcx, rdi ; return value slot\r\n call Widget::Widget(bool) ; create a widget\r\n mov rax, rdi ; and return it\r\n<\/pre>\nOne possible fix is to return a pointer to the Widget:<\/p>\n
struct Doodad\r\n{\r\n \u27e6 ... \u27e7\r\n\r\n Widget*<\/span> GetWidget()\r\n {\r\n if (!m_widget) {\r\n m_widget = std::make_unique<Widget>();\r\n }\r\n return m_widget.get();\r\n }\r\n};\r\n\r\nvoid Sample()\r\n{\r\n Doodad doodad;\r\n\r\n \/\/ Demand-create the widget and increment its counter.\r\n doodad.GetWidget()-><\/span>Increment();\r\n\r\n \/\/ This assertion no longer fires<\/span>\r\n ASSERT(doodad.GetWidget()-><\/span>m_count > 0);\r\n}\r\n<\/pre>\nAnother option is to return a reference to the Widget.<\/p>\n
struct Doodad\r\n{\r\n \u27e6 ... \u27e7\r\n\r\n Widget&<\/span> GetWidget()\r\n {\r\n if (!m_widget) {\r\n m_widget = std::make_unique<Widget>();\r\n }\r\n return *m_widget<\/span>;\r\n }\r\n};\r\n\r\nvoid Sample()\r\n{\r\n Doodad doodad;\r\n\r\n \/\/ Demand-create the widget and increment its counter.\r\n doodad.GetWidget().Increment(); \/\/ no change to callers<\/span>\r\n\r\n \/\/ This assertion no longer fires<\/span>\r\n ASSERT(doodad.GetWidget().m_count > 0);\r\n}\r\n<\/pre>\nWhile we’re at it, let’s make that constructor explicit to remove the implicit conversion.<\/p>\n
struct Widget\r\n{\r\n explicit<\/span> Widget(bool debugMode = false);\r\n\r\n int m_count = 0;\r\n void Increment() { ++m_count; }\r\n};\r\n<\/pre>\n","protected":false},"excerpt":{"rendered":"C++ language backward compatibility makes legal what you had hoped wasn’t.<\/p>\n","protected":false},"author":1069,"featured_media":110434,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[1],"tags":[25],"class_list":["post-111228","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
C++ language backward compatibility makes legal what you had hoped wasn’t.<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/111228","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/users\/1069"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/comments?post=111228"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/111228\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media\/110434"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media?parent=111228"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=111228"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=111228"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}