Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compilation error C4576 with C++ and MSVC 2019 (19.28.29913) #1885

Merged
merged 1 commit into from
Dec 27, 2021

Conversation

hwmaier
Copy link
Contributor

@hwmaier hwmaier commented Aug 31, 2021

When using this SDK with MS C++ 2019 (19.28.29913), including the header file az_json.h into a C++ file yields the following error:

azure/core/az_json.h(292): error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax
azure/core/az_json.h(600): error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax

Example code to reproduce:

#include <azure/core/az_json.h>

int main(void)
{
}

compile above using this command line from VS 2019 command prompt:

cl -I sdk/inc /std:c++latest main.cpp

Removing the redundant C-style type casts in the initialisers fixes this compilation error.

@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Aug 31, 2021
@ghost
Copy link

ghost commented Aug 31, 2021

Thank you for your contribution hwmaier! We will review the pull request and get back to you soon.

@ahsonkhan
Copy link
Contributor

We might have this pattern used elsewhere within the SDK. @hwmaier was this the only instance you found?

I wonder if we should add this scenario of consuming the C SDK in a C++ app, using a C++ compiler within CI to catch such issues going forward. cc @danieljurek

@ahsonkhan
Copy link
Contributor

@CaseyCarter and @barcharcraz should the MSVC compiler be complaining about this? I don't fully understand the issue here, maybe you can shed some light on the topic.

@hwmaier
Copy link
Contributor Author

hwmaier commented Sep 1, 2021

Hi @ahsonkhan , I only used so far the JSON components of the SDK, so I cannot comment if other header files have similar issues with VS 2019. Interestingly gcc 9 and gcc 10 do not complain about it.

While I understand that there is also a C++ SDK, there are instances where the C API has to be consumed in a C++ application. In our scenario our C++ application consumes Azure NetXDuo which contains this SDK as a subcomponent,

From my understanding the cast is redundant and unnecessary, hence I suggest to remove it.

@CaseyCarter
Copy link

@CaseyCarter and @barcharcraz should the MSVC compiler be complaining about this? I don't fully understand the issue here, maybe you can shed some light on the topic.

This construct - (T) { /* ... stuff to initialize a T ... */ } - is a C "compound literal". It is an lvalue expression with type T that denotes an unnamed object initialized with the provided braced-initializer. The unnamed object has static storage duration if declared outside of any function body, and automatic storage duration if declared in an enclosing function block (C99 or later 6.5.2.5).

Compound literals are not valid syntax in C++, but accepted by some C++ compilers (clearly not MSVC) as an extension except (IIRC) that those C++ compilers treat the compound literal as a prvalue expression of type T exactly equivalent to T{ /* ... initializer stuff ... */ }.

There seems to be absolutely no point to the use of compound literals here - why initialize by copying from a compound literal when we can initialize directly?

@RickWinter RickWinter merged commit 00d858c into Azure:main Dec 27, 2021
@ahsonkhan
Copy link
Contributor

ahsonkhan commented Jan 18, 2022

@CaseyCarter should we make this a coding guideline and fix the pattern across the SDK?

We have other instances in our client libraries, like:

return (az_iot_hub_client_options){ .module_id = AZ_SPAN_EMPTY,

options = (az_storage_blobs_blob_client_options) {

cc @danewalton, @antkmsft

Also, @danieljurek is there any way for us to add CI leg for this MSVC version so we always stay green, even though it is a C++ compiler, and I am not sure if this scenario is one we actively intend to support for Embedded C.

I will file issues to track this change shortly :)

@CaseyCarter
Copy link

While I understand that there is also a C++ SDK, there are instances where the C API has to be consumed in a C++ application.

This is news to me, and I assume @barcharcraz as well? We would not have encouraged using constructs outside the common subset of C and C++ in C SDKs if consuming those SDKs from C++ is a requirement.

@ahsonkhan
Copy link
Contributor

While I understand that there is also a C++ SDK, there are instances where the C API has to be consumed in a C++ application. In our scenario our C++ application consumes Azure NetXDuo which contains this SDK as a subcomponent,

@hwmaier - just so I understand it better, can you share more info about your scenario and application use case? What drove the decision to write your app in C++ instead of C, while using things like NetXDuo? And, with this fix, are you unblocked now? Are you able to consume Azure NetxDuo (and indirectly this SDK) successfully?

@hwmaier
Copy link
Contributor Author

hwmaier commented Jan 18, 2022

@ahsonkhan This is a surprising question. C++ is a modern programming language with OOP concepts and a template class standard library which implement modern design patterns like containers and algorithms. The productivity benefits of using C++ over plain old C in software design are tremendous and I believe well accepted in the software industry. NetXDuo is not available as C++ version as most RTOS aren't and that is the benefit of C++ that you can consume C libraries and C API without problem.

@ahsonkhan
Copy link
Contributor

Using C++ over C due to the productivity and modern language features for app development, certainly make sense :)
I was just curious what your specific motivations were, if there was anything unique to your scenario. Thanks for clarifying.

As an aside: Which platforms do you expect your C++ app to run? Are they intended for desktop platforms/OSes like Windows, Linux, MacOS, or for particular embedded devices? And, other than MSVC, what are the compilers you use for building, if any?

@hwmaier
Copy link
Contributor Author

hwmaier commented Jan 19, 2022

@ahsonkhan To be quite frank, we use C++ in all our products, embedded or not. We remove exception handling for embedded platform as this is expensive and some of the more complex containers. We have written C++ wrappers for ThreadX/NetXDuo's RTOS API which mimic the C++ STL API. So we have code which is multiplatform. Embedded on Azure RTOS using gcc and MSVC and MSVC on Windows and gcc on Linux. Azure RTOS with MSVC we mainly use to develop as we run the whole RTOS stack on Windows which gives us a very fast compile/run/debug cycle. Essentially we can develop most of the embedded code on Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants