[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

rnk added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:3889 + if (Context.getTargetInfo().getCXXABI().isMicrosoft() && + hasFunctionProto(D) && isFunctionOrMethodVariadic(D)) { Why is this warning dependent on the ABI? GCC has a similar

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT added a comment. Thanks! I don't have the means to check this in myself. Repository: rC Clang https://reviews.llvm.org/D44646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT updated this revision to Diff 141055. DHowett-MSFT added a comment. Switched from `i686-windows` to `i686-unknown-windows-msvc` as per @compnerd's suggestion. Based on the prior discussion around MSVC's definition of `va_{start,end,arg}` and how it is not a valid implementation

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT marked 5 inline comments as done. DHowett-MSFT added a comment. @compnerd CL warns for `__forceinline` variadics in C code as well: test.c(1): warning C4714: function 'void hello(int,...)' marked as __forceinline not inlined Repository: rC Clang

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT added inline comments. Comment at: test/Sema/ms-forceinline-on-variadic.cpp:14 +__builtin_va_end(ap); +} + efriedma wrote: > compnerd wrote: > > DHowett-MSFT wrote: > > > compnerd wrote: > > > > Would be nice to have a second test that uses the

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

efriedma added inline comments. Comment at: test/Sema/ms-forceinline-on-variadic.cpp:14 +__builtin_va_end(ap); +} + compnerd wrote: > DHowett-MSFT wrote: > > compnerd wrote: > > > Would be nice to have a second test that uses the Microsoft definitions > > >

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

compnerd added inline comments. Comment at: test/Sema/ms-forceinline-on-variadic.cpp:14 +__builtin_va_end(ap); +} + DHowett-MSFT wrote: > compnerd wrote: > > Would be nice to have a second test that uses the Microsoft definitions > > (`char *` and the

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT added inline comments. Comment at: test/Sema/ms-forceinline-on-variadic.cpp:1 +// RUN: %clang_cc1 -emit-llvm -o - -triple i686-windows -verify -fms-extensions %s \ +// RUN:| FileCheck %s compnerd wrote: > Personally, I prefer the fully

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT added a comment. In https://reviews.llvm.org/D44646#1042347, @efriedma wrote: > The compiler shouldn't inline functions which call va_start, whether or not > they're marked always_inline. That should work correctly, I think, at least > on trunk. (See

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

compnerd added a comment. What happens in the case that you have a variadic in C code marked with `__forceinline`? Does that also cause a warning with MSVC? Comment at: test/Sema/ms-forceinline-on-variadic.cpp:1 +// RUN: %clang_cc1 -emit-llvm -o - -triple i686-windows

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

efriedma added a comment. The compiler shouldn't inline functions which call va_start, whether or not they're marked always_inline. That should work correctly, I think, at least on trunk. (See https://reviews.llvm.org/D42556 .) If you want to warn anyway, that's okay. Repository: rC

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT updated this revision to Diff 138995. DHowett-MSFT added a comment. Fixed formatting. Repository: rC Clang https://reviews.llvm.org/D44646 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclAttr.cpp test/Sema/ms-forceinline-on-variadic.cpp Index:

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

smeenai added a reviewer: compnerd. smeenai added a comment. @DHowett-MSFT the reviewers look fine to me. Reid is the code owner for clang's MSVC compat support. David doesn't work on this stuff directly anymore, I think, but he's still pretty active in code reviews for it. I'm adding Saleem,

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT added a comment. Apologies if I've chosen the wrong set of reviewers. Repository: rC Clang https://reviews.llvm.org/D44646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

DHowett-MSFT created this revision. DHowett-MSFT added reviewers: rnk, majnemer. DHowett-MSFT added a project: clang. Herald added subscribers: cfe-commits, eraman. The MSVC compiler rejects `__forceinline` on variadic functions with the following warning (at /https://reviews.llvm.org/W4):