Page MenuHome

Utils: Add macro for C++ default arguments in C headers
ClosedPublic

Authored by Julian Eisel (Severin) on Apr 14 2022, 6:52 PM.

Details

Summary

Default arguments are a nice quality-of-life feature in C++. It's annoying if these can't be used in C++ files, just because the header with the function declaration still needs to be C compatible. This macro adds the default argument if the translation unit is compiled in C++. Otherwise, the argument has to be passed explicitly.

Usage looks like this:

void some_func(int required_arg, Something *defaulted_arg CPP_ARG_DEFAULT(nullptr));
/* Or: */
void some_other_func(int required_arg, int defaulted_arg CPP_ARG_DEFAULT(0));

D14653 which uses this compiles on all platforms: https://builder.blender.org/admin/#/builders/18/builds/369

I used this twice in D14653, but I can see this being useful in plenty of other cases. Should the header be C++ only eventually, it's easy to just replace the usages of the macro there. This is easier to clean up than wrapper functions (which have to be identified manually) or explicit arguments littered around the code base. And since wrapper functions are annoying to add anyway, we end up just passing explicit arguments a lot even though there could be a sensible default. Put differently this should encourage default argument use, which might be a good thing. So while this adds a slightly ugly syntax, it should help us move to more readable code eventually.

Diff Detail

Repository
rB Blender

Event Timeline

Julian Eisel (Severin) requested review of this revision.Apr 14 2022, 6:52 PM
Julian Eisel (Severin) created this revision.
Julian Eisel (Severin) edited the summary of this revision. (Show Details)Apr 14 2022, 6:54 PM
Julian Eisel (Severin) added a project: Core.
Julian Eisel (Severin) edited the summary of this revision. (Show Details)Apr 14 2022, 6:56 PM
Julian Eisel (Severin) edited the summary of this revision. (Show Details)

I also tested D14653 with this little dummy patch, to check if it also compiles if the function is called from C too:

diff --git a/source/blender/editors/interface/interface_layout.c b/source/blender/editors/interface/interface_layout.c
index c1bb2ed6d18..ea8ea6fcb9f 100644
--- a/source/blender/editors/interface/interface_layout.c
+++ b/source/blender/editors/interface/interface_layout.c
@@ -5541,6 +5541,10 @@ void ui_layout_add_but(uiLayout *layout, uiBut *but)
     but->context = layout->context;
     but->context->used = true;
   }
+  const PointerRNA *ptr = UI_but_context_ptr_get(but, "id", NULL);
+  if (ptr) {
+    printf("test\n");
+  }
 
   if (layout->emboss != UI_EMBOSS_UNDEFINED) {
     but->emboss = layout->emboss;

Worked fine on Apple Clang.

I don't mind this, even though if it doesn't directly apply to C usage, it still helps there by adding additional context regarding expected values, in the case of D14653, the comments around UI_but_context_ptr_get are somewhat lacking, before it wasn't clear if type had to be a valid pointer or not, now seeing the CPP_ARG_DEFAULT(nullptr) on there tells me "You may safely cram a nullptr in there" without having to go look at the actual implementation, and that's probably worth more to me than any syntactic sugar it adds for C++ callers.

This revision is now accepted and ready to land.Apr 14 2022, 7:22 PM
Julian Eisel (Severin) edited the summary of this revision. (Show Details)Apr 25 2022, 5:56 PM