r242730 - Ignore the "novtable" declspec when not using the Microsoft C++ ABI.

Bob Wilson bob.wilson at apple.com
Tue Jul 21 09:41:32 PDT 2015


> On Jul 21, 2015, at 5:22 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> 
> On Mon, Jul 20, 2015 at 6:57 PM, Bob Wilson <bob.wilson at 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

> 
>> 
>> 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 at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits





More information about the cfe-commits mailing list