abseil-cpp icon indicating copy to clipboard operation
abseil-cpp copied to clipboard

Add `!define(__clang__)` for GNU compiler matching

Open jrandolf opened this issue 3 years ago • 3 comments

The original PR (https://github.com/abseil/abseil-cpp/pull/1285) left out a condition that did not enforce matching the GNU compiler. The problem comes from clang also defining the __GNUC__ macro, so clang would escape through the first condition.

jrandolf avatar Oct 07 '22 11:10 jrandolf

Somehow this is breaking a test when using our production Clang compiler. It may take me a while to look into why this is happening. Sorry about this.

derekmauro avatar Oct 10 '22 13:10 derekmauro

Have you verified the production compiler has the patch against the clang bug linked in the comments?

jrandolf avatar Oct 10 '22 18:10 jrandolf

Have you verified the production compiler has the patch against the clang bug linked in the comments?

Yes I have.

derekmauro avatar Oct 10 '22 19:10 derekmauro

Any updates on this?

AZero13 avatar Feb 14 '23 15:02 AZero13

Any updates on this?

I actually tried this recently and it is still and issue with our production compiler. I still don't know why and I haven't dug any deeper.

derekmauro avatar Feb 14 '23 16:02 derekmauro

My theory here is that while LLVM fixed the codegen for __builtin_nan, that isn't actually what we are calling. We are calling std::nan, and as far as I can tell, libc++ just calls into glibc. https://github.com/llvm/llvm-project/blob/6c7acc6409cb13c7df264a3d07e2ff2b6119fc40/libcxx/include/cmath#L341

I think a change to use __builtin_nan if available will allow this change to work.

derekmauro avatar May 15 '23 19:05 derekmauro