pico-sdk icon indicating copy to clipboard operation
pico-sdk copied to clipboard

Configuration option to disable `static inline` on functions

Open KizzyCode opened this issue 3 years ago • 9 comments

Would it be possible to replace the static inlines with a configurable attribute that can be disabled if necessary?

Background

We are using a Rust library to implement the main logic. The lib defines a pub extern "C" fn rust_entry() which gets called from the C main function and then takes over control and calls back into the Pico-SDK where necessary (e.g. for malloc/free, GPIO etc.)

This concept works amazingly well – except for one problem: Since a lot of the SDK's functions are static, we cannot link against them. Our current workaround is to implement a 1:1 C-wrapper for all the static functions we need – however this gets quite cumbersome over time.

A possible naive solution could be sth. like

#ifndef PICO_DISABLE_STATIC_INLINE
    #define __pico_static_inline static inline
#else
    #define __pico_static_inline
#endif

and then using __pico_static_inline on all functions (if it is not crucial that they are inlined of course).

I know that our setup is probably a little bit exotic and I'm completely fine if this issue gets closed 😅 However IMO this could ease the burden to write an application in language-of-your-choice, because if you can link back against the SDK, you suddenly don't need an entire native toolchain/SDK in language-of-your-choice anymore to get stuff running.

KizzyCode avatar Aug 23 '22 23:08 KizzyCode

I've never written any Rust myself, and I don't know enough about the C SDK to answer your question, but have you had a look at https://github.com/rp-rs ? ping @thejpster

lurch avatar Aug 24 '22 00:08 lurch

Yes, I know about rp-rs; nevertheless tyvm for the suggestion :D

There are a few reasons why we decided to stick with the official SDK if possible (e.g. pretty stable, quick bugfixes for breaking issues, has a lot of built-in features etc. – we found that even if another toolchain suits the needs we have now there is a significant risk that we well run into problems/missing features in the future).

KizzyCode avatar Aug 24 '22 00:08 KizzyCode

I ran into a similar problem providing access to the SDK routines from BBC BASIC. My solution was to write some Python code which reads through the SDK header files and automatically generates wrapper code such as:

#include "pico/types.h"

uint64_t stub_to_us_since_boot (absolute_time_t t)
    {
    return to_us_since_boot (t);
    }

void stub_update_us_since_boot (absolute_time_t *t, uint64_t us_since_boot)
    {
    update_us_since_boot (t, us_since_boot);
    }

The stub_ functions are then callable. The BBC BASIC function look-up (SYS) hides the stub_ prefix from the user.

Since the wrapper functions are automatically generated they can be reproduced from the latest SDK at every build.

My problem is with LWIP which makes extensive use of C #define macros. These cannot be automatically stubbed as the parameter and return types are unknown.

Memotech-Bill avatar Aug 24 '22 13:08 Memotech-Bill

@Memotech-Bill Uhh, this looks pretty nice – I'll definitively take a closer look at it, this could be a good workaround!

KizzyCode avatar Aug 24 '22 13:08 KizzyCode

This is potentially useful for people who want to make a static lib of the SDK (although #882 also helps)

kilograham avatar Aug 24 '22 14:08 kilograham

@Memotech-Bill , really nice idea, I am fighting the same issue with my freepascal port of the pico headers, I ended up re-implementing lots of the static inlines with pascal code.

michael-ring avatar Aug 24 '22 14:08 michael-ring

My 2p: Rather than disabling static wholesale, it seems to make sense to go through the API and decide what is public (and hence something you are careful not to break without warning people) and what is internal only. The internal APIs should not have extern linkage (i.e. remain static), and the public API should have extern linkage (i.e. not be marked static).

If application developers rely on APIs that are internal only, you should consider stabilizing them and making them public.

If the issue is that the static inline functions are trivial things defined (and not just declared) in the header file (ugh), then this kind of macro is probably the right work-around (also, don't do that and consider enabling LTO instead).

thejpster avatar Aug 24 '22 16:08 thejpster

Yeah this is just referring to public API functions which happen to implemented inline for code size/speed reasons.

And now i give it more than a moments thought, having something to generate is better, because either a) you remove the static as well as the inline, in which case you'll get multiple-definitions b) you leave the static, and you get duplicate code, but also no public callable version

kilograham avatar Aug 24 '22 17:08 kilograham

I'm a little bit torn between both options... On one side, generating something is much easier and faster; on the other side – @thejpster has a point: Using static inline within headers for public APIs is non-idiomatic C and breaks FFI compatibility.

Furthermore I'm wondering if static inline is necessary? Doing LTO is probably a much better approach and has the additional advantage that it can be tweaked for either performance or code size, whereas static inline always optimizes for performance.

For those interested: As a stopgap I've created a tiny helper to generate no-static no-inline bindings for the static inline functions. It's a bit cursed because it uses regex to parse the headers, but it works surprisingly well 😅 There are also pregenerated bindings in generated/.

KizzyCode avatar Sep 13 '22 00:09 KizzyCode