From 7b3fe969927731c69ba4d8a428442e1e191f49b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Sat, 13 Jun 2020 22:29:52 +0300 Subject: [PATCH] [clang] Don't emit warn_cxx_ms_struct when MSBitfields is enabled globally This diagnostic (which defaults to an error, added in 95833f33bda6c92e746e0b0007b69c2c30bfc693) was intended to clearly point out cases where the C++ ABI won't match the Microsoft C++ ABI, for cases when this is enabled via a pragma over a region of code. The MSVC compatible struct layout feature can also be enabled via a compiler option (-mms-bitfields). If enabled that way, one essentially can't compile any C++ code unless also building with -Wno-incompatible-ms-struct (which GCC doesn't support, and projects developed with GCC aren't setting). For the MinGW target, it's expected that the C++ ABI won't match the MSVC one, if this option is used for getting the struct layout to match MSVC. Differential Revision: https://reviews.llvm.org/D81794 --- clang/lib/Sema/SemaDeclCXX.cpp | 6 +++++- clang/test/SemaCXX/ms_struct.cpp | 14 ++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index f1ade752e2fe..a2b669c99125 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -6773,7 +6773,11 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl *Record) { // headers, sweeping up a bunch of types that the project doesn't // really rely on MSVC-compatible layout for. We must therefore // support "ms_struct except for C++ stuff" as a secondary ABI. - if (Record->isMsStruct(Context) && + // Don't emit this diagnostic if the feature was enabled as a + // language option (as opposed to via a pragma or attribute), as + // the option -mms-bitfields otherwise essentially makes it impossible + // to build C++ code, unless this diagnostic is turned off. + if (Record->isMsStruct(Context) && !Context.getLangOpts().MSBitfields && (Record->isPolymorphic() || Record->getNumBases())) { Diag(Record->getLocation(), diag::warn_cxx_ms_struct); } diff --git a/clang/test/SemaCXX/ms_struct.cpp b/clang/test/SemaCXX/ms_struct.cpp index 414b56b491c6..122819c3eead 100644 --- a/clang/test/SemaCXX/ms_struct.cpp +++ b/clang/test/SemaCXX/ms_struct.cpp @@ -1,8 +1,11 @@ -// RUN: %clang_cc1 -fsyntax-only -Wno-error=incompatible-ms-struct -verify -triple i686-apple-darwin9 -std=c++11 %s -// RUN: %clang_cc1 -fsyntax-only -Wno-error=incompatible-ms-struct -verify -triple armv7-apple-darwin9 -std=c++11 %s +// RUN: %clang_cc1 -fsyntax-only -DTEST_FOR_WARNING -Wno-error=incompatible-ms-struct -verify -triple i686-apple-darwin9 -std=c++11 %s +// RUN: %clang_cc1 -fsyntax-only -DTEST_FOR_WARNING -Wno-error=incompatible-ms-struct -verify -triple armv7-apple-darwin9 -std=c++11 %s // RUN: %clang_cc1 -fsyntax-only -DTEST_FOR_ERROR -verify -triple armv7-apple-darwin9 -std=c++11 %s +// RUN: %clang_cc1 -fsyntax-only -DNO_PRAGMA -mms-bitfields -verify -triple armv7-apple-darwin9 -std=c++11 %s +#ifndef NO_PRAGMA #pragma ms_struct on +#endif struct A { unsigned long a:4; @@ -12,7 +15,7 @@ struct A { struct B : public A { #ifdef TEST_FOR_ERROR // expected-error@-2 {{ms_struct may not produce Microsoft-compatible layouts for classes with base classes or virtual functions}} -#else +#elif defined(TEST_FOR_WARNING) // expected-warning@-4 {{ms_struct may not produce Microsoft-compatible layouts for classes with base classes or virtual functions}} #endif unsigned long c:16; @@ -27,7 +30,7 @@ static_assert(__builtin_offsetof(B, d) == 12, struct C { #ifdef TEST_FOR_ERROR // expected-error@-2 {{ms_struct may not produce Microsoft-compatible layouts for classes with base classes or virtual functions}} -#else +#elif defined(TEST_FOR_WARNING) // expected-warning@-4 {{ms_struct may not produce Microsoft-compatible layouts for classes with base classes or virtual functions}} #endif virtual void foo(); @@ -36,3 +39,6 @@ struct C { static_assert(__builtin_offsetof(C, n) == 8, "long long field in ms_struct should be 8-byte aligned"); +#if !defined(TEST_FOR_ERROR) && !defined(TEST_FOR_WARNING) +// expected-no-diagnostics +#endif