microsoft / cpp_client_telemetry

1DS C++ SDK
Apache License 2.0
87 stars 49 forks source link

No exceptions build #472

Open maxgolov opened 4 years ago

maxgolov commented 4 years ago

In a few interesting environments:

It'd make sense to totally avoid using exceptions. For that reason the SDK is providing MATSDK_THROW macro, that allows to either crash (std::terminate) the app, or transform the failure into some sort of error code.

I believe a few recent changes might have introduced exceptions, such as throwing without using MATSDK_THROW macro. That results in having to build with exceptions. We do want to maintain ability to build without exceptions, so these recent changes must be carefully reviewed.

maxgolov commented 4 years ago

@larvacea - I can try to validate this with Gnu C++ and Clang on Mac, i.e. how it all works if we try the no-exceptions build.

Would you be able to check if some of the existing Android layer code can use a macro instead of try / catch / throw? (as a lower priority item). Historically I remember that some apps, like Skype, provided an option of building without exceptions, with throw being a function call instead of the actual standard library exception flow.

larvacea commented 4 years ago

The existing Android layer code does not need to throw C++ exceptions, so that's the simple, painless approach. The STL will likely throw exceptions. If you feel it would be helpful, I can annotate methods as noexcept to make STL exceptions turn into terminate().

maxgolov commented 4 years ago

Let's see if react would work as-is in static build with matching runtime ( #471 ), then solving no-except would probably be of lower priority. I'd like to enable as many teams as possible :) React seems like an interesting / useful scenario to enable.

maxgolov commented 4 years ago

I'm removing BUG label from it - since we generally have the SDK working fine in most environments. No-exceptions may require a bit of a buildflag / build infra changes, and although seems like easy on surface - this may have implications, like we most likely NOT gonna be adding no exceptions into CI loop. I'll try to spend a few hours on cleaning this up, testing on Mac and/or Linux Desktop.

maxgolov commented 4 years ago

Martin's fix in #475 cleans-up the Android side. I'd like to see if we can specify a new build option at build.sh script level to force the no-except for POSIX-alike systems. On Windows we're usually (and normally) fine to live with exceptions in place.