[PATCH] Move optional into the experimental namespace (first bit of the TSes)

[Howard Hinnant]

Thanks, please commit.

Howard

On Nov 14, 2013, at 2:03 PM, Marshall Clow <mclow.lists at gmail.com> wrote:

> 
> On Nov 12, 2013, at 3:56 PM, Marshall Clow <mclow.lists at gmail.com> wrote:
> 
>> On Nov 12, 2013, at 3:33 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> 
>>> I assume the intent is to implement this paper:
>>> 
>>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3793.html
>>> 
>>> So, assuming that include/experimental/optional was copied from include/optional (the diff doesn't say where the 'old' lines come from here)…
>> 
>> Yes.
>> 
>>> You included the inline namespace in the library synopsis comment; it shouldn't be there.
>> 
>> Ok.
> 
> Fixed.
> 
>> 
>>> bad_optional_access should be in namespace std::experimental, not in namespace std.
>> 
>> I went back and forth about that. I agree - it _should_ be there.
>> 
>> However:
>> 	The destructor for bad_optional_access lives in libc++.dylib. (due to a peculiarity in the Itanium ABI).
>> 	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.
> 
> I put the bad_optional_access into "std::experimental" (but not into "std::experimental::__library_fundamentals_v1")
> 
>> 
>>> For headers in include/support, the source files are in src/support. Should optional.cpp be moved to src/experimental/optional.cpp?
>> 
>> Good question. 
>> Header folder structure is an interface.
>> Source folder structure is not.
>> 
>> I don't see that this is necessary; but if other people really want it, I have no objection to changing it.
> 
> I did not change this.
> 
>>> Bikeshedding on the name __TSLibraryFundamentals:
>>>  * 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.
>> 
>> Yes! Crap - there was supposed to be a _v1 on the end of that.
>> 
>>>  * Other library-internal names in libc++ seem to use __lower_case_names
>>> Maybe __library_fundamentals_1?
>> 
>> That's fine - I can live with either one.
>> 
> Changed to __library_fundamentals_v1.
> 
> I also cleaned up a bunch of the tests with "using" directives.
> New patch attached.
> Thanks for the review.
> 
> -- Marshall
> 
> Marshall Clow     Idio Software   <mailto:mclow.lists at gmail.com>
> 
> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).
>         -- Yu Suzuki
> <optional-ts2.patch>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits