On Tue, Jul 21, 2015 at 12:41 PM, Bob Wilson <bob.wil...@apple.com> wrote: > >> On Jul 21, 2015, at 5:22 AM, Aaron Ballman <aa...@aaronballman.com> wrote: >> >> On Mon, Jul 20, 2015 at 6:57 PM, Bob Wilson <bob.wil...@apple.com> wrote: >>> Author: bwilson >>> Date: Mon Jul 20 17:57:31 2015 >>> New Revision: 242730 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=242730&view=rev >>> Log: >>> Ignore the "novtable" declspec when not using the Microsoft C++ ABI. >>> >>> Clang used to silently ignore __declspec(novtable). It is implemented >>> now, but leaving the vtable uninitialized does not work when using the >>> Itanium ABI, where the class layout for complex class hierarchies is >>> stored in the vtable. It might be possible to honor the novtable >>> attribute in some simple cases and either report an error or ignore >>> it in more complex situations, but it’s not clear if that would be >>> worthwhile. There is also value in having a simple and predictable >>> behavior, so this changes clang to simply ignore novtable when not using >>> the Microsoft C++ ABI. >>> >>> Added: >>> cfe/trunk/test/SemaCXX/ms-unsupported.cpp >>> Modified: >>> cfe/trunk/include/clang/Basic/Attr.td >>> cfe/trunk/include/clang/Basic/Attributes.h >>> cfe/trunk/include/clang/Sema/AttributeList.h >>> cfe/trunk/lib/Basic/Attributes.cpp >>> cfe/trunk/lib/Lex/PPMacroExpansion.cpp >>> cfe/trunk/lib/Parse/ParseDecl.cpp >>> cfe/trunk/lib/Parse/ParseDeclCXX.cpp >>> cfe/trunk/lib/Sema/AttributeList.cpp >>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >>> cfe/trunk/test/Parser/MicrosoftExtensions.cpp >>> cfe/trunk/test/SemaCXX/ms-novtable.cpp >>> cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp >>> >>> Modified: cfe/trunk/include/clang/Basic/Attr.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Basic/Attr.td (original) >>> +++ cfe/trunk/include/clang/Basic/Attr.td Mon Jul 20 17:57:31 2015 >>> @@ -241,14 +241,18 @@ def COnly : LangOpt<"CPlusPlus", 1>; >>> class TargetArch<list<string> arches> { >>> list<string> Arches = arches; >>> list<string> OSes; >>> + list<string> CXXABIs; >>> } >>> def TargetARM : TargetArch<["arm", "thumb"]>; >>> +def TargetMips : TargetArch<["mips", "mipsel"]>; >>> def TargetMSP430 : TargetArch<["msp430"]>; >>> def TargetX86 : TargetArch<["x86"]>; >>> def TargetWindows : TargetArch<["x86", "x86_64", "arm", "thumb"]> { >>> let OSes = ["Win32"]; >>> } >>> -def TargetMips : TargetArch<["mips", "mipsel"]>; >>> +def TargetMicrosoftCXXABI : TargetArch<["x86", "x86_64", "arm", "thumb"]> { >>> + let CXXABIs = ["Microsoft"]; >>> +} >>> >>> class Attr { >>> // The various ways in which an attribute can be spelled in source >>> @@ -1820,7 +1824,7 @@ def TypeTagForDatatype : InheritableAttr >>> >>> // Microsoft-related attributes >>> >>> -def MSNoVTable : InheritableAttr { >>> +def MSNoVTable : InheritableAttr, >>> TargetSpecificAttr<TargetMicrosoftCXXABI> { >>> let Spellings = [Declspec<"novtable">]; >>> let Subjects = SubjectList<[CXXRecord]>; >>> let Documentation = [MSNoVTableDocs]; >> >> Can you also update MSNoVTableDocs to mention this new behavior? >> >> Thanks! >> >> ~Aaron > > Sure. r242800
Thank you, commit looks good! ~Aaron > >> >>> >>> Modified: cfe/trunk/include/clang/Basic/Attributes.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attributes.h?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Basic/Attributes.h (original) >>> +++ cfe/trunk/include/clang/Basic/Attributes.h Mon Jul 20 17:57:31 2015 >>> @@ -11,7 +11,7 @@ >>> #define LLVM_CLANG_BASIC_ATTRIBUTES_H >>> >>> #include "clang/Basic/LangOptions.h" >>> -#include "llvm/ADT/Triple.h" >>> +#include "clang/Basic/TargetInfo.h" >>> >>> namespace clang { >>> >>> @@ -31,7 +31,7 @@ enum class AttrSyntax { >>> /// \brief Return the version number associated with the attribute if we >>> /// recognize and implement the attribute specified by the given >>> information. >>> int hasAttribute(AttrSyntax Syntax, const IdentifierInfo *Scope, >>> - const IdentifierInfo *Attr, const llvm::Triple &T, >>> + const IdentifierInfo *Attr, const TargetInfo &Target, >>> const LangOptions &LangOpts); >>> >>> } // end namespace clang >>> >>> Modified: cfe/trunk/include/clang/Sema/AttributeList.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Sema/AttributeList.h (original) >>> +++ cfe/trunk/include/clang/Sema/AttributeList.h Mon Jul 20 17:57:31 2015 >>> @@ -16,11 +16,11 @@ >>> #define LLVM_CLANG_SEMA_ATTRIBUTELIST_H >>> >>> #include "clang/Basic/SourceLocation.h" >>> +#include "clang/Basic/TargetInfo.h" >>> #include "clang/Basic/VersionTuple.h" >>> #include "clang/Sema/Ownership.h" >>> #include "llvm/ADT/PointerUnion.h" >>> #include "llvm/ADT/SmallVector.h" >>> -#include "llvm/ADT/Triple.h" >>> #include "llvm/Support/Allocator.h" >>> #include <cassert> >>> >>> @@ -464,7 +464,7 @@ public: >>> bool hasVariadicArg() const; >>> bool diagnoseAppertainsTo(class Sema &S, const Decl *D) const; >>> bool diagnoseLangOpts(class Sema &S) const; >>> - bool existsInTarget(const llvm::Triple &T) const; >>> + bool existsInTarget(const TargetInfo &Target) const; >>> bool isKnownToGCC() const; >>> >>> /// \brief If the parsed attribute has a semantic equivalent, and it would >>> >>> Modified: cfe/trunk/lib/Basic/Attributes.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Attributes.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Basic/Attributes.cpp (original) >>> +++ cfe/trunk/lib/Basic/Attributes.cpp Mon Jul 20 17:57:31 2015 >>> @@ -4,8 +4,8 @@ >>> using namespace clang; >>> >>> int clang::hasAttribute(AttrSyntax Syntax, const IdentifierInfo *Scope, >>> - const IdentifierInfo *Attr, const llvm::Triple &T, >>> - const LangOptions &LangOpts) { >>> + const IdentifierInfo *Attr, const TargetInfo >>> &Target, >>> + const LangOptions &LangOpts) { >>> StringRef Name = Attr->getName(); >>> // Normalize the attribute name, __foo__ becomes foo. >>> if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__")) >>> >>> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original) >>> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Mon Jul 20 17:57:31 2015 >>> @@ -1633,13 +1633,13 @@ void Preprocessor::ExpandBuiltinMacro(To >>> Value = FeatureII->getBuiltinID() != 0; >>> } else if (II == Ident__has_attribute) >>> Value = hasAttribute(AttrSyntax::GNU, nullptr, FeatureII, >>> - getTargetInfo().getTriple(), getLangOpts()); >>> + getTargetInfo(), getLangOpts()); >>> else if (II == Ident__has_cpp_attribute) >>> Value = hasAttribute(AttrSyntax::CXX, ScopeII, FeatureII, >>> - getTargetInfo().getTriple(), getLangOpts()); >>> + getTargetInfo(), getLangOpts()); >>> else if (II == Ident__has_declspec) >>> Value = hasAttribute(AttrSyntax::Declspec, nullptr, FeatureII, >>> - getTargetInfo().getTriple(), getLangOpts()); >>> + getTargetInfo(), getLangOpts()); >>> else if (II == Ident__has_extension) >>> Value = HasExtension(*this, FeatureII); >>> else { >>> >>> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original) >>> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Jul 20 17:57:31 2015 >>> @@ -387,7 +387,7 @@ bool Parser::ParseMicrosoftDeclSpecArgs( >>> // If the attribute isn't known, we will not attempt to parse any >>> // arguments. >>> if (!hasAttribute(AttrSyntax::Declspec, nullptr, AttrName, >>> - getTargetInfo().getTriple(), getLangOpts())) { >>> + getTargetInfo(), getLangOpts())) { >>> // Eat the left paren, then skip to the ending right paren. >>> ConsumeParen(); >>> SkipUntil(tok::r_paren); >>> >>> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original) >>> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Mon Jul 20 17:57:31 2015 >>> @@ -3612,7 +3612,7 @@ bool Parser::ParseCXX11AttributeArgs(Ide >>> // If the attribute isn't known, we will not attempt to parse any >>> // arguments. >>> if (!hasAttribute(AttrSyntax::CXX, ScopeName, AttrName, >>> - getTargetInfo().getTriple(), getLangOpts())) { >>> + getTargetInfo(), getLangOpts())) { >>> // Eat the left paren, then skip to the ending right paren. >>> ConsumeParen(); >>> SkipUntil(tok::r_paren); >>> >>> Modified: cfe/trunk/lib/Sema/AttributeList.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AttributeList.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/AttributeList.cpp (original) >>> +++ cfe/trunk/lib/Sema/AttributeList.cpp Mon Jul 20 17:57:31 2015 >>> @@ -17,6 +17,7 @@ >>> #include "clang/AST/DeclTemplate.h" >>> #include "clang/AST/Expr.h" >>> #include "clang/Basic/IdentifierTable.h" >>> +#include "clang/Basic/TargetInfo.h" >>> #include "clang/Sema/SemaInternal.h" >>> #include "llvm/ADT/SmallString.h" >>> #include "llvm/ADT/StringSwitch.h" >>> @@ -155,7 +156,7 @@ struct ParsedAttrInfo { >>> bool (*DiagAppertainsToDecl)(Sema &S, const AttributeList &Attr, >>> const Decl *); >>> bool (*DiagLangOpts)(Sema &S, const AttributeList &Attr); >>> - bool (*ExistsInTarget)(const llvm::Triple &T); >>> + bool (*ExistsInTarget)(const TargetInfo &Target); >>> unsigned (*SpellingIndexToSemanticSpelling)(const AttributeList &Attr); >>> }; >>> >>> @@ -195,8 +196,8 @@ bool AttributeList::isTypeAttr() const { >>> return getInfo(*this).IsType; >>> } >>> >>> -bool AttributeList::existsInTarget(const llvm::Triple &T) const { >>> - return getInfo(*this).ExistsInTarget(T); >>> +bool AttributeList::existsInTarget(const TargetInfo &Target) const { >>> + return getInfo(*this).ExistsInTarget(Target); >>> } >>> >>> bool AttributeList::isKnownToGCC() const { >>> >>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Jul 20 17:57:31 2015 >>> @@ -4583,7 +4583,7 @@ static void ProcessDeclAttribute(Sema &S >>> // which do not apply to the current target architecture are treated as >>> // though they were unknown attributes. >>> if (Attr.getKind() == AttributeList::UnknownAttribute || >>> - !Attr.existsInTarget(S.Context.getTargetInfo().getTriple())) { >>> + !Attr.existsInTarget(S.Context.getTargetInfo())) { >>> S.Diag(Attr.getLoc(), Attr.isDeclspecAttribute() >>> ? diag::warn_unhandled_ms_attribute_ignored >>> : diag::warn_unknown_attribute_ignored) >>> >>> Modified: cfe/trunk/test/Parser/MicrosoftExtensions.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/test/Parser/MicrosoftExtensions.cpp (original) >>> +++ cfe/trunk/test/Parser/MicrosoftExtensions.cpp Mon Jul 20 17:57:31 2015 >>> @@ -1,4 +1,4 @@ >>> -// RUN: %clang_cc1 %s -triple i386-mingw32 -std=c++14 -fsyntax-only >>> -Wno-unused-getter-return-value -Wno-unused-value -Wmicrosoft -verify >>> -fms-extensions -fms-compatibility -fdelayed-template-parsing >>> +// RUN: %clang_cc1 %s -triple i386-pc-win32 -std=c++14 -fsyntax-only >>> -Wno-unused-getter-return-value -Wno-unused-value -Wmicrosoft -verify >>> -fms-extensions -fms-compatibility -fdelayed-template-parsing >>> >>> /* Microsoft attribute tests */ >>> [repeatable][source_annotation_attribute( Parameter|ReturnValue )] >>> >>> Modified: cfe/trunk/test/SemaCXX/ms-novtable.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-novtable.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/test/SemaCXX/ms-novtable.cpp (original) >>> +++ cfe/trunk/test/SemaCXX/ms-novtable.cpp Mon Jul 20 17:57:31 2015 >>> @@ -1,4 +1,4 @@ >>> -// RUN: %clang_cc1 %s -fsyntax-only -verify -fms-extensions -Wno-microsoft >>> -std=c++11 >>> +// RUN: %clang_cc1 -triple i386-pc-win32 %s -fsyntax-only -verify >>> -fms-extensions -Wno-microsoft -std=c++11 >>> >>> struct __declspec(novtable) S {}; >>> enum __declspec(novtable) E {}; // expected-warning{{'novtable' attribute >>> only applies to classes}} >>> >>> Added: cfe/trunk/test/SemaCXX/ms-unsupported.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-unsupported.cpp?rev=242730&view=auto >>> ============================================================================== >>> --- cfe/trunk/test/SemaCXX/ms-unsupported.cpp (added) >>> +++ cfe/trunk/test/SemaCXX/ms-unsupported.cpp Mon Jul 20 17:57:31 2015 >>> @@ -0,0 +1,5 @@ >>> +// RUN: %clang_cc1 -triple x86_64-pc-windows-gnu %s -fsyntax-only -verify >>> -fms-extensions -Wno-microsoft -std=c++11 >>> + >>> +// "novtable" is ignored except with the Microsoft C++ ABI. >>> +// MinGW uses the Itanium C++ ABI so check that it is ignored there. >>> +struct __declspec(novtable) S {}; // expected-warning{{__declspec >>> attribute 'novtable' is not supported}} >>> >>> Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=242730&r1=242729&r2=242730&view=diff >>> ============================================================================== >>> --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original) >>> +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Mon Jul 20 17:57:31 2015 >>> @@ -1885,7 +1885,8 @@ static void GenerateHasAttrSpellingStrin >>> } >>> } >>> >>> - // It is assumed that there will be an llvm::Triple object named T >>> within >>> + // It is assumed that there will be an llvm::Triple object >>> + // named "T" and a TargetInfo object named "Target" within >>> // scope that can be used to determine whether the attribute exists in >>> // a given target. >>> std::string Test; >>> @@ -1902,6 +1903,7 @@ static void GenerateHasAttrSpellingStrin >>> } >>> Test += ")"; >>> >>> + // If the attribute is specific to particular OSes, check those. >>> std::vector<std::string> OSes; >>> if (!R->isValueUnset("OSes")) { >>> Test += " && ("; >>> @@ -1916,6 +1918,21 @@ static void GenerateHasAttrSpellingStrin >>> Test += ")"; >>> } >>> >>> + // If one or more CXX ABIs are specified, check those as well. >>> + if (!R->isValueUnset("CXXABIs")) { >>> + Test += " && ("; >>> + std::vector<std::string> CXXABIs = >>> + R->getValueAsListOfStrings("CXXABIs"); >>> + for (auto AI = CXXABIs.begin(), AE = CXXABIs.end(); AI != AE; >>> ++AI) { >>> + std::string Part = *AI; >>> + >>> + Test += "Target.getCXXABI().getKind() == TargetCXXABI::" + Part; >>> + if (AI + 1 != AE) >>> + Test += " || "; >>> + } >>> + Test += ")"; >>> + } >>> + >>> // If this is the C++11 variety, also add in the LangOpts test. >>> if (Variety == "CXX11") >>> Test += " && LangOpts.CPlusPlus11"; >>> @@ -1962,6 +1979,7 @@ void EmitClangAttrHasAttrImpl(RecordKeep >>> } >>> } >>> >>> + OS << "const llvm::Triple &T = Target.getTriple();\n"; >>> OS << "switch (Syntax) {\n"; >>> OS << "case AttrSyntax::GNU:\n"; >>> OS << " return llvm::StringSwitch<int>(Name)\n"; >>> @@ -2464,7 +2482,7 @@ static std::string GenerateLangOptRequir >>> } >>> >>> static void GenerateDefaultTargetRequirements(raw_ostream &OS) { >>> - OS << "static bool defaultTargetRequirements(const llvm::Triple &) {\n"; >>> + OS << "static bool defaultTargetRequirements(const TargetInfo &) {\n"; >>> OS << " return true;\n"; >>> OS << "}\n\n"; >>> } >>> @@ -2533,6 +2551,20 @@ static std::string GenerateTargetRequire >>> Test += ")"; >>> } >>> >>> + // Test for the C++ ABI, if specified. >>> + if (!R->isValueUnset("CXXABIs")) { >>> + Test += " && ("; >>> + std::vector<std::string> CXXABIs = >>> R->getValueAsListOfStrings("CXXABIs"); >>> + for (auto I = CXXABIs.begin(), E = CXXABIs.end(); I != E; ++I) { >>> + std::string Part = *I; >>> + Test += "Target.getCXXABI().getKind() == TargetCXXABI::" + Part; >>> + if (I + 1 != E) >>> + Test += " || "; >>> + FnName += Part; >>> + } >>> + Test += ")"; >>> + } >>> + >>> // If this code has already been generated, simply return the previous >>> // instance of it. >>> static std::set<std::string> CustomTargetSet; >>> @@ -2540,7 +2572,8 @@ static std::string GenerateTargetRequire >>> if (I != CustomTargetSet.end()) >>> return *I; >>> >>> - OS << "static bool " << FnName << "(const llvm::Triple &T) {\n"; >>> + OS << "static bool " << FnName << "(const TargetInfo &Target) {\n"; >>> + OS << " const llvm::Triple &T = Target.getTriple();\n"; >>> OS << " llvm::Triple::ArchType Arch = T.getArch();\n"; >>> if (UsesOS) >>> OS << " llvm::Triple::OSType OS = T.getOS();\n"; >>> >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits