Page MenuHomePhabricator

eina: add Eina_Value helper that convert efficiently to a target native C type.
ClosedPublic

Authored by segfaultxavi on Jan 16 2019, 6:28 PM.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
cedric created this revision.Jan 16 2019, 6:28 PM
segfaultxavi requested changes to this revision.Jan 17 2019, 2:12 AM

Ugh, the amount of code repetition! Are you sure yo do not want to use a macro for that?
The macro can include the docs also, and Doxygen will pick them up just fine.

src/lib/eina/eina_inline_value_util.x
990

Shouldn't this be 1.22?

1001

*conversion

This revision now requires changes to proceed.Jan 17 2019, 2:12 AM

Ugh, the amount of code repetition! Are you sure yo do not want to use a macro for that?

I am not sure. I am afraid of the hard to read part of it.

The macro can include the docs also, and Doxygen will pick them up just fine.

That was something I didn't know. So if I have a macro that generate function, it will actually generate independent Doxygen comment for each of them ? How does that work ? Do you have example?

cedric updated this revision to Diff 18570.Jan 17 2019, 4:39 PM

Rebase and fix documentation.

SanghyeonLee accepted this revision.Jan 18 2019, 3:25 AM

if you have plan to update the patch with macro by @segfaultxavi's comment's I'll be waiting it.
the patch is buildable and test passed, all good so far.

That was something I didn't know. So if I have a macro that generate function, it will actually generate independent Doxygen comment for each of them ? How does that work ? Do you have example?

I knew it could be done, but I had to work a bit to have it done:

test.h:

/// @brief For internal use only.
/// @hideinitializer
#define PRINT_CHAR(ch,doc) \
/** @brief doc
 */                        \
void print_ ## ch () {     \
  printf( # ch "\n");      \
}

PRINT_CHAR(a, "Prints an A")
PRINT_CHAR(b, "Prints a B")

The above code defines two functions print_a() and print_b() both with documentation text, which is controlled from the macro parameters.
In order for Doxygen to actually expand the macros and use the expanded comments, some Doxygen options need to be set:

Doxyfile: (I am showing the relevant lines)

#---------------------------------------------------------------------------
# Configuration options related to the preprocessor
#---------------------------------------------------------------------------
ENABLE_PREPROCESSING   = YES
MACRO_EXPANSION        = YES
EXPAND_ONLY_PREDEF     = YES
EXPAND_AS_DEFINED      = PRINT_CHAR

All these are the values we are already currently using, except EXPAND_AS_DEFINED! Awesome!
I would have liked to hide the PRINT_CHAR macro completely from the output, but I could not manage. It it is not shown, the macros are not expanded either. The above code is the nicest I could manage.

In summary, I think it is worth trying this because otherwise you are adding 500 lines of extremely repeated code :/
Some other portions of this header could also benefit from this approach.

cedric updated this revision to Diff 18624.Jan 18 2019, 12:18 PM

Rebase and refactor with nice magic Macro.

SanghyeonLee accepted this revision.Jan 21 2019, 2:52 AM

I think this macro looks nice work but seems @segfaultxavi again review once about it? I 'll leave the decision on you @cedric

I am still reviewing it. It does not seem to work for me. Have you guys been able to see the html output?

segfaultxavi commandeered this revision.Jan 22 2019, 3:54 AM
segfaultxavi edited reviewers, added: cedric; removed: segfaultxavi.

It'll be easier if I do it myself :)

This revision is now accepted and ready to land.Jan 22 2019, 3:54 AM

Update Doxyfile so .x files are also processed.
Add new functions into a group.
Cleanup comments.

Remove duplicated entry in Doxyfile.in

This revision was automatically updated to reflect the committed changes.
SanghyeonLee edited the summary of this revision. (Show Details)Jan 25 2019, 2:32 AM