This is an archive of the discontinued LLVM Phabricator instance.

MS Compat: interpose vadefs.h to fix definitions of _crt_va_{start,end,arg} (PR21247)
ClosedPublic

Authored by hans on Oct 14 2014, 2:14 PM.

Diff Detail

Event Timeline

hans updated this revision to Diff 14889.Oct 14 2014, 2:14 PM
hans retitled this revision from to MS Compat: interpose vadefs.h to fix definitions of _crt_va_{start,end,arg} (PR21247).
hans updated this object.
hans edited the test plan for this revision. (Show Details)
hans added reviewers: rnk, majnemer.
hans added subscribers: Unknown Object (MLST), hansw.
rnk added inline comments.Oct 14 2014, 3:19 PM
lib/Headers/vadefs.h
25

I'd wordsmith this to "Only include this if we're aiming for MSVC compatibility."

33

Should we only do this if __has_include(<vadefs.h>) ?

35

I think we should do "#ifndef _crt_va_start #undef _crt_va_start" to avoid warnings with -Wsystem-headers.

hans added inline comments.Oct 14 2014, 3:34 PM
lib/Headers/vadefs.h
25

Done.

33

I think we should always do it. If the user is including vadefs.h, she's expecting to get the system one, and we should just always try to forward to it.

35

Done.

hans updated this revision to Diff 14899.Oct 14 2014, 3:34 PM

Addressing rnk's comments.

I tried this on a small program with -Wsystem-headers, and it was clean.

rnk accepted this revision.Oct 14 2014, 3:40 PM
rnk edited edge metadata.

lgtm

This revision is now accepted and ready to land.Oct 14 2014, 3:40 PM
hans closed this revision.Oct 14 2014, 3:46 PM
hans updated this revision to Diff 14901.

Closed by commit rL219740 (authored by @hans).