flipperzero-firmware icon indicating copy to clipboard operation
flipperzero-firmware copied to clipboard

SDK: Include guards in the public headers of services

Open CookiePLMonster opened this issue 1 year ago • 4 comments

Description of the feature you're suggesting.

This is an idea/request related strictly to the SDK/development experience, with no changes to the compiled code.

While the Furi API gets included all at once, this is not the case for services that need to be included on demand - for example, gui/view.h. This creates a possible usability issue for things like C++ wrappers for service types.

Consider the example of Windows Implementation Libraries providing unique_ptr-like wrappers for WinAPI types. This header file can be included multiple times after the include introducing the type the user wishes to wrap, and determines whether the specific type should be wrapped through _INC_xxx defines. Those are defined by the header include guards themselves, for example stdio.h:

#pragma once
#ifndef _INC_STDIO // include guard for 3rd party interop
#define _INC_STDIO

My suggestion is to roll out these include guards alongside #pragma once to public service API headers, in a similar vein to the above. This allows for generic libraries making use of service types without having to include them inside the header and potentially polluting the namespace. In my case, the intended use case would be to create RAII wrappers identical to the example from WIL I linked above.

Anything else?

An alternate approach to this request could be to include RAII types inside the headers themselves if __cplusplus is defined, but that only covers this one potential use case, while there may be more.

CookiePLMonster avatar Jan 15 '24 12:01 CookiePLMonster

It is not clear to me why #pragma once and header guards would be needed together. Can you provide an example usecase?

DrZlo13 avatar Jan 16 '24 10:01 DrZlo13

Can you provide an example usecase?

The link from WIL I provided mirrors the use case I have in mind. In this case, assuming those header guards get added to headers, imagine a theoretical furi_cpp.h that looks like:

// _VIEW_INC comes from gui/view.h being included
#if defined(_VIEW_INC) && !defined(_FURI_CPP_VIEW_INC)
#define _FURI_CPP_VIEW_INC
typedef unique_ptr<View, decltype(&::view_free), ::view_free> unique_view;
#endif

// _WIDGET_INC comes from gui/widget.h being included
#if defined(_WIDGET_INC ) && !defined(_FURI_CPP_WIDGET_INC)
#define _FURI_CPP_WIDGET_INC
typedef unique_ptr<Widget, decltype(&::widget_free), ::widget_free> unique_widget;
#endif

Without header guards (or any other define signifying what specific headers have been included), a theoretical furi_cpp.h would have to be split per-service to avoid referencing undefined symbols.

#pragma once should be kept since AFAIK is special cased by compilers and is more optimal over the header guards approach. Guards would be there only as markers that the file has been included. I suppose they could just be

#ifndef _VIEW_INC
#define _VIEW_INC
#endif // _VIEW_INC

instead of being full-blown header guards, but the two are essentially equivalent.

CookiePLMonster avatar Jan 16 '24 10:01 CookiePLMonster

That is a lot of work that we are not ready to do without significant reason.

Significant reason for us:

  • Feature must be used by large group of our developers
  • Feature must not break anything else

As far as I see this is going to be very niche thing.

skotopes avatar Jan 23 '24 07:01 skotopes

Yep, I don't disagree. It's just something that would be "nice to have" to enable users to implement RAII wrappers for Furi types. Perhaps it'd be quicker to implement those directly instead of exposing header guards.

CookiePLMonster avatar Jan 23 '24 10:01 CookiePLMonster