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

[Reid Kleckner via Phabricator via llvm-commits]

rnk requested changes to this revision.
rnk added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/sanitizer_common/sanitizer_internal_defs.h:42
+# define WEAK(ReturnType, Name, Params)                                        \
+  WIN_WEAK_ALIAS(Name, WEAK_DEF_NAME(Name))                                    \
+  extern "C" ReturnType Name Params;                                           \
----------------
After looking at D28597 and D28598, I realized how this usage of WIN_WEAK_ALIAS makes it so that every weak declaration now introduces a `#pragma comment(linker, "/alternatename:...")`. That seems bad. In particular, I don't like how including `sanitizer_interface_internal.h` in D28598 is used for the side effect of providing aliases for the sanitizer coverage callbacks.

It might be nicer to provide separate macros for declaring and defining weak functions. On Windows, weak function declarations wouldn't have anything special added. Then, at the definition, they would provide the pragma and the default definition.


================
Comment at: lib/sanitizer_common/sanitizer_internal_defs.h:60
+//  + Always a default implementation must me provided.
+//  + When linking dinamically with a library (dll), only the default
+//  implementation is provided (fun__def()). If some unit overrides the function
----------------
typo on "dynamically"


Repository:
  rL LLVM

https://reviews.llvm.org/D28596