[cfe-commits] [libcxx] r132129 - in /libcxx/trunk: include/exception include/initializer_list include/limits include/new include/typeinfo src/exception.cpp src/new.cpp src/typeinfo.cpp

[Howard Hinnant]

Thanks for double checking this for me.  Comments below.

On May 26, 2011, at 2:42 PM, Jonathan Sauer wrote:

> Hello,
> 
>>    // access functions
>> -    void rethrow_nested [[noreturn]] () const;
>> -    exception_ptr nested_ptr() const;
>> +    [[noreturn]] void rethrow_nested() const;
>> +    exception_ptr nested_ptr() const noexcept;
>> };
>> 
>> -template <class T> void throw_with_nested [[noreturn]] (T&& t);
>> +template <class T> [[noreturn]] void throw_with_nested(T&& t);
> 
> Is there a particular reason why you are using "[[noreturn]]" instead of "_ATTRIBUTE(noreturn)", as you
> do below with <unexpected> and <terminate>? Especially since the latter supports compilation with
> compilers that don't support attributes (yet).
> 
> Although I don't know if these compilers are relevant anymore.
> 
> Also, above you are using "noexcept", whereas below sometimes "_NOEXCEPT" is used? This as well as the
> above seems inconsistent to me.

The modifications above are to documentation, not code (it's a synopsis in a comment).

> 
>> [...]
>> typedef void (*unexpected_handler)();
>> -_LIBCPP_VISIBLE unexpected_handler set_unexpected(unexpected_handler) throw();
>> -_LIBCPP_VISIBLE unexpected_handler get_unexpected() throw();
>> +_LIBCPP_VISIBLE unexpected_handler set_unexpected(unexpected_handler) _NOEXCEPT;
>> +_LIBCPP_VISIBLE unexpected_handler get_unexpected() _NOEXCEPT;
>> _ATTRIBUTE(noreturn) _LIBCPP_VISIBLE void unexpected();
>> [...]
>> -    nested_exception();
>> -//     nested_exception(const nested_exception&) throw() = default;
>> -//     nested_exception& operator=(const nested_exception&) throw() = default;
> 
> In __config, _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS is always defined. Shouldn't the "= default" only be used if
> defaulted functions are actually supported by the compiler?

Yes, but those are comments too.

> 
> Also in __config, there is (lines 94ff):
> 
> | //#if !__has_feature(cxx_alias_templates)
> | #define _LIBCPP_HAS_NO_TEMPLATE_ALIASES
> | //#endif
> 
> even though the check for feature "cxx_alias_templates" is now supported.

Ah, thanks for reminding me about that.  I tried to turn it on a couple of weeks ago and it didn't work, even though the flag said it did.  I don't recall the problem.  But it is high time to check into that again, and if I find a problem, I'll submit a bug.

Howard