[PATCH] D28525: [compiler-rt] Use macros to simplify weak alias for Windows and add some documentation.
Reid Kleckner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 10 13:00:12 PST 2017
rnk added a comment.
Thanks for the refactoring! I'd been thinking of doing it for a while.
================
Comment at: lib/asan/asan_globals_win.h:1
//===-- asan_globals_win.h --------------------------------------*- C++ -*-===//
//
----------------
I think with your change, this header is no longer needed. We can replace all instances of `ASAN_LINK_GLOBALS_WIN()` with `WIN_FORCE_LINK(__asan_dso_reg_hook)`.
================
Comment at: lib/sanitizer_common/sanitizer_win_defs.h:17
+#if !defined(_MSC_VER)
+#error "this file is Windows-only, and uses MSVC pragmas"
+#endif
----------------
The existing pattern in sanitizer_linux.h and sanitizer_mac.h is to allow the header to be included everywhere but ifdef out the contents. I think this reduced ifdefs and is in general consistent with how Kostya likes to do things in the sanitizer libraries. Let's do that. So, please change this to `#if SANITIZER_WINDOWS`.
================
Comment at: lib/ubsan/ubsan_flags.cc:20
#include "sanitizer_common/sanitizer_flag_parser.h"
+#include "sanitizer_common/sanitizer_win_defs.h"
----------------
kubabrecka wrote:
> I think this needs to be included only on Windows, because sanitizer_win_defs.h errors out if _MCS_VER is not defined.
Let's fix the header to make it easy to include instead.
Repository:
rL LLVM
https://reviews.llvm.org/D28525
More information about the llvm-commits
mailing list