react-native: New props infrastructure is not portable

an alternative approach is to use an immediate lambda instead of a statement expression:

case [&]{ /* ... */ return something; }(): /* ... */

Looks like the fix has landed: https://github.com/facebook/react-native/commit/a142a784733bc7e37042a7e4d3cdbeff99aa1fa7. @rshest didn’t test this on MSVC but it should work in theory. If there’s an issue you can let us know here.

@chiaramooney I’d say just put out a PR on this repo for it, and then we’ll see if builds successfully in our CI.

ok, looks like that syntax is a language extension in GCC (and clang implements it too) but it is not standard C++:

https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

@JoshuaGross could we write this in a way that is portable? maybe move the “statement expression” inside of a #define instead of just the pragma

yes we’re on cpp17

Yeah we’re not on C++20 yet either, so I’ll have to find something that works with just constexpr. I’ll play around a little bit. If y’all get to it faster, let me know if MSVC is fine with just removing the pragmas.

thanks @JoshuaGross for taking a look! Yeah I’m using a macro to do consteval for cpp20 and constexpr otherwise; we don’t yet use cpp20, so this would downgrade to constexpr for us.

@asklar I should be able to change the macro into a consteval function to enforce its evaluation at compile-time. Let me know if there are any concerns with doing that for MSVC. It looks like you’re using consteval in https://github.com/microsoft/react-native-windows/pull/9841/files as well (I had not seen this PR previously) so maybe that’s an acceptable solution here; I’ll give it a shot.

if we remove the pragma, I imagine msvc might produce a warning if the right warning level flag is set, perhaps @chiaramooney can try it. is there a reason why we need the extra variable? why not just say:

#define CONSTEXPR_RAW_PROPS_KEY_HASH(s)    (folly::hash::fnv32_buf(s, sizeof(s) - 1))

is the problem that folly’s fnv32_buf functions aren’t constexpr? take a look at the PR I made a reference to originally and see if that helps, that does everything in constexpr without any funky macros

@asklar do you know if in React Native Windows we’ll get a warning or error if we had this code as-is without the pragma? The issue with Clang was that this code could introduce variables len and hash which could shadow variables with the same name in an outer scope. If React Native Windows has MSVC configured to produce an error for this situation we’ll have to add the MSVC pragma equivalent. If so, do you know what that is?