I can commit sometime today; thanks.

Address review comments by Marshall

In D50736#1200774, @hubert.reinterpretcast wrote:

In D50736#1200761, @mclow.lists wrote:

Is this test that's being added libc++ specific, or would it apply to other implementations as well?

The test can apply to other implementations as well (although I am not sure how the initializer_list include behaves under C++03 on other implementations). Is there some other place to put such tests?

It seems that test/std/numerics/rand/rand.eng/rand.eng.mers/ may be the place to add the test. I am looking into it.

In D50736#1200761, @mclow.lists wrote:

Is this test that's being added libc++ specific, or would it apply to other implementations as well?

The test can apply to other implementations as well (although I am not sure how the initializer_list include behaves under C++03 on other implementations). Is there some other place to put such tests?

Use cp -R, which is sufficient instead of cp -RPp

I'll commit with cp -R tomorrow then; thanks.

Maybe cp -R is sufficient? cp -RPp was the just-to-be-safe "minimal change".

Ping

Looks good to me.

In D33833#789436, @kuang_he wrote:

Can we get this patch reviewed by any chance?

@kuang_he; it is customary to "ping". In this case, "Ping 2".

Remove stray ellipsis treatment entirely

In D33339#759797, @rsmith wrote:

In D33339#759146, @hubert.reinterpretcast wrote:

The check-all target passes even if the ellipsis-after-declarator-id disambiguation as a declarator is removed entirely.

[...]

So, on the whole, the stray ellipsis treatment is both too complicated and not complicated enough.

I think if we want to keep it, the way to do that would be to carry on through the disambiguation process and treat it as a tiebreaker (that's what we do, for instance, if we see an undeclared identifier in a position where we're looking for a type). I'm not convinced that's worthwhile, especially since our existing testcases do not need this disambiguation rule, but perhaps we could remove the stray ellipsis treatment entirely for now and reconsider adding it back if we find poor diagnostics result from it later?

In D33339#759125, @rsmith wrote:

Should I assume our "misplaced ellipsis" diagnostic requires that we disambiguate the ill-formed ellipsis-after-declarator-id as a declarator in some cases? If so, do we have tests for that somewhere?

Tests for the diagnostics are in clang/test/FixIt/fixit-cxx0x.cpp and clang/test/Parser/cxx11-templates.cpp.
The check-all target passes even if the ellipsis-after-declarator-id disambiguation as a declarator is removed entirely. The disambiguation process is not needed in many cases, and—when it is—can choose the declarative interpretation for other reasons.

Has it been discussed whether this is something to be addressed in the optimizer as opposed to the front-end?

Address review comments; update to revision 294580

Ping 2?

Ping!

Address review comments; update to revision 292996

Friendly ping...

In D25480#567552, @ahatanak wrote:

__builtin_fpclassify takes five int arguments followed by one last argument that is of floating point type. Do you know if there is a way to specify the last one argument is a floating point rather than using '.'?

There is already code in clang/lib/Sema/SemaChecking.cpp to generate diag::err_typecheck_call_invalid_unary_fp.

A test should probably be added as well.

In D24011#529508, @timshen wrote:

If there seems to be few applications are depending on this optimization, I wonder if we can turn off the constant folding for now, and fix APFloat later if someone really cares about it?

Just wanted to note that the front-end would still encounter the issue with APFloat, and I would be concerned if the (front-end) constant expression evaluation for ppc_fp128 is disabled entirely.

LGTM.

LGTM.

Make FixedSizeStorage accessible to test code

Make FixedSizeStorageOwner accessible to test code

Replace call to LLVM_CONSTEXPR function in constant expression context

Avoid MSVC C4099: replace 'struct' with 'class'

Address review comments: add tests, access control, Doxygen

In D22668#499164, @aaron.ballman wrote:

I don't suppose there's a way to test these changes, is there?

It's a utility class (which is not even used yet). I am not aware of testing for the ADTs, etc. aside from using them internally. Perhaps I should mark the change as NFC?

This looks straightforward. @mclow.lists, are you okay with me committing this for Jason?

Minor concerns noted. I'd be okay with this patch, but would like someone else to give the okay (or indicate what issues need to be addressed).

I believe tests are necessary.