<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Nov 12, 2013, at 3:56 PM, Marshall Clow <<a href="mailto:mclow.lists@gmail.com">mclow.lists@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Nov 12, 2013, at 3:33 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr"><div>I assume the intent is to implement this paper:</div><div><br></div><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3793.html">http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3793.html</a><br>
<div><br></div><div>So, assuming that include/experimental/optional was copied from include/optional (the diff doesn't say where the 'old' lines come from here)…</div></div></blockquote><div><br></div>Yes.</div><div><br><blockquote type="cite"><div dir="ltr">You included the inline namespace in the library synopsis comment; it shouldn't be there.</div></blockquote><div><br></div>Ok.</div></div></blockquote><div><br></div>Fixed.</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br><blockquote type="cite"><div dir="ltr">
<div>bad_optional_access should be in namespace std::experimental, not in namespace std.</div></div></blockquote><div><br></div>I went back and forth about that. I agree - it _should_ be there.</div><div><br></div><div>However:</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>The destructor for bad_optional_access lives in libc++.dylib. (due to a peculiarity in the Itanium ABI).</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>If I put bad_optional_access in namespace std, then users do not need a new dylib when optional moves from std::experimental to std.</div></div></blockquote><div><br></div>I put the bad_optional_access into "std::experimental" (but not into "std::experimental::__library_fundamentals_v1")</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div><blockquote type="cite"><div dir="ltr">For headers in include/support, the source files are in src/support. Should optional.cpp be moved to src/experimental/optional.cpp?</div></blockquote><div><br></div><div>Good question. </div><div>Header folder structure is an interface.</div><div>Source folder structure is not.</div><div><br></div><div>I don't see that this is necessary; but if other people really want it, I have no objection to changing it.</div></div></div></blockquote><div><br></div>I did not change this.</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div dir="ltr"><div>Bikeshedding on the name __TSLibraryFundamentals:</div><div> * Should this contain a version number? We may want an ABI break at some point, and it might be nicer to have always had the version number in the name.</div></div></blockquote><div><br></div>Yes! Crap - there was supposed to be a _v1 on the end of that.</div></div></blockquote><div><blockquote type="cite"><br></blockquote></div><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div dir="ltr"><div> * Other library-internal names in libc++ seem to use __lower_case_names</div><div>Maybe __library_fundamentals_1?</div></div></blockquote><div><br></div>That's fine - I can live with either one.</div><div><br><blockquote type="cite"></blockquote></div></div></blockquote></div><div>Changed to __library_fundamentals_v1.</div><div><br></div><div>I also cleaned up a bunch of the tests with "using" directives.</div><div>New patch attached.</div><div>Thanks for the review.</div><div><br></div></div><div apple-content-edited="true">
<span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: 'Lucida Grande'; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; ">-- Marshall<br><br>Marshall Clow Idio Software <<a href="mailto:mclow.lists@gmail.com">mailto:mclow.lists@gmail.com</a>><br><br>A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).<br> -- Yu Suzuki</span>
</div>
</body></html>