[PATCH] D28596: [compiler-rt] General definition for weak functions.

[Marcos Pividori via Phabricator via llvm-commits]

mpividori added a comment.

In https://reviews.llvm.org/D28596#649214, @kubabrecka wrote:

> > To avoid duplicated code, I will define a macro WEAK_DEF(), so we can reduce that code to:
> > 
> >   WEAK_DEF(void, fun, ()) {
> >     // Default implementation
> >   }
> > 
> > Would you agree on this point? Would you do it differently?
>
> I don't like that this is not a "standard" function declaration.  It's not obvious that this declares a function.  My editor/IDE will not recognize this as a function, and "Go to definition" and such will not work.  Is there really no way of keeping the syntax of...:
>
>   WHATEVER_MACROS_NECESSARY
>   void fun() MAYBE_OTHER_MACROS {
>     ...code...
>   }
>   
>
> ?


Hi @kubabrecka ,
Thanks for your reply.
I think the macro name `WEAK_DEF()` makes it explicit that we are defining a function. Also, I am writing the components in the same order: return value, function name, parameters and function body. I think it is easy to understand by any user, so the only problem is the editor.
I can't keep the syntax as you suggest, because I need to define a function with a different name adding the suffix "__def" and also define a weak alias, and declare the original function. So, definitely this can not be accomplished in that way.
Also, is the same approach used for the definition of the macros:  `INTERCEPTOR()` , `DECLARE_REAL()` (in: `interception/interception.h`), for `RECOVERABLE()` and `UNRECOVERABLE()` (in: `ubsan/ubsan_handlers.h`), `LLVM_SYMBOLIZER_INTERCEPTOR*()` (in: `sanitizer_common/symbolizer/sanitizer_wrappers`),  etc. So, this is also used in other parts of compiler-rt repository.
As weak functions are a small portion of the definitions, I think this won't generate many problems to users working with an editor that doesn't expand the macros.
Also, I think the advantages are considerably more relevant than that disadvantage. We can define weak functions that will work in all platforms with a unique macro, without boilerplate code. If we want to continue supporting Linux, Darwin and Windows, I think this is useful.
Would you agree?
Thanks


https://reviews.llvm.org/D28596