r195805 - Remove 'DistinctSpellings' support from Attr.td and change its only user to
Jordan Rose
jordan_rose at apple.com
Wed Dec 4 10:04:31 PST 2013
Yeah, the ownership attributes were an experiment from a while back, but neither we nor Andrew (the original contributor) ever really got time to do anything with them AFAIK. Ted, Anna, and I have tossed around the idea of a general "analyzer_annotate" attribute, which would be more structured than "annotate" but still only very lightly validated, so that new checkers could add new "attributes" without having to mess with the rest of Clang.
For now, I guess we keep the ownership attributes...though if no one is using them, I suppose we could consider deprecating them and remove them in a future release. We haven't tested them at all with any of the changes in MallocChecker in the last year, at least. I really don't know what Andrew's old company is doing, though.
Jordan
On Nov 26, 2013, at 17:46 , Richard Smith <richard-llvm at metafoo.co.uk> wrote:
> Author: rsmith
> Date: Tue Nov 26 19:46:48 2013
> New Revision: 195805
>
> URL: http://llvm.org/viewvc/llvm-project?rev=195805&view=rev
> Log:
> Remove 'DistinctSpellings' support from Attr.td and change its only user to
> look at the attribute spelling instead. The 'ownership_*' attributes should
> probably be split into separate *Attr classes, but that's more than I wanted to
> do here.
>
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.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=195805&r1=195804&r2=195805&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Tue Nov 26 19:46:48 2013
> @@ -131,8 +131,6 @@ class Attr {
> bit SemaHandler = 1;
> // Set to true for attributes that are completely ignored.
> bit Ignored = 0;
> - // Set to true if each of the spellings is a distinct attribute.
> - bit DistinctSpellings = 0;
> // Set to true if the attribute's parsing does not match its semantic
> // content. Eg) It parses 3 args, but semantically takes 4 args. Opts out of
> // common attribute error checking.
> @@ -662,12 +660,18 @@ def Override : InheritableAttr {
> def Ownership : InheritableAttr {
> let Spellings = [GNU<"ownership_holds">, GNU<"ownership_returns">,
> GNU<"ownership_takes">];
> - let DistinctSpellings = 1;
> - let Args = [EnumArgument<"OwnKind", "OwnershipKind",
> - ["ownership_holds", "ownership_returns", "ownership_takes"],
> - ["Holds", "Returns", "Takes"]>,
> - StringArgument<"Module">, VariadicUnsignedArgument<"Args">];
> - let HasCustomParsing = 1;
> + let Accessors = [Accessor<"isHolds", [GNU<"ownership_holds">]>,
> + Accessor<"isReturns", [GNU<"ownership_returns">]>,
> + Accessor<"isTakes", [GNU<"ownership_takes">]>];
> + let AdditionalMembers = [{
> + enum OwnershipKind { Holds, Returns, Takes };
> + OwnershipKind getOwnKind() const {
> + return isHolds() ? Holds :
> + isTakes() ? Takes :
> + Returns;
> + }
> + }];
> + let Args = [IdentifierArgument<"Module">, VariadicUnsignedArgument<"Args">];
> }
>
> def Packed : InheritableAttr {
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=195805&r1=195804&r2=195805&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Nov 26 19:46:48 2013
> @@ -1546,34 +1546,26 @@ static void handleOwnershipAttr(Sema &S,
> return;
> }
>
> - // Figure out our Kind, and check arguments while we're at it.
> - OwnershipAttr::OwnershipKind K;
> - switch (AL.getKind()) {
> - case AttributeList::AT_ownership_takes:
> - K = OwnershipAttr::Takes;
> + // Figure out our Kind.
> + OwnershipAttr::OwnershipKind K =
> + OwnershipAttr(AL.getLoc(), S.Context, 0, 0, 0,
> + AL.getAttributeSpellingListIndex()).getOwnKind();
> +
> + // Check arguments.
> + switch (K) {
> + case OwnershipAttr::Takes:
> + case OwnershipAttr::Holds:
> if (AL.getNumArgs() < 2) {
> S.Diag(AL.getLoc(), diag::err_attribute_too_few_arguments) << 2;
> return;
> }
> break;
> - case AttributeList::AT_ownership_holds:
> - K = OwnershipAttr::Holds;
> - if (AL.getNumArgs() < 2) {
> - S.Diag(AL.getLoc(), diag::err_attribute_too_few_arguments) << 2;
> - return;
> - }
> - break;
> - case AttributeList::AT_ownership_returns:
> - K = OwnershipAttr::Returns;
> -
> + case OwnershipAttr::Returns:
> if (AL.getNumArgs() > 2) {
> S.Diag(AL.getLoc(), diag::err_attribute_too_many_arguments) << 1;
> return;
> }
> break;
> - default:
> - // This should never happen given how we are called.
> - llvm_unreachable("Unknown ownership attribute");
> }
>
> if (!isFunction(D) || !hasFunctionProto(D)) {
> @@ -1582,11 +1574,15 @@ static void handleOwnershipAttr(Sema &S,
> return;
> }
>
> - StringRef Module = AL.getArgAsIdent(0)->Ident->getName();
> + IdentifierInfo *Module = AL.getArgAsIdent(0)->Ident;
>
> // Normalize the argument, __foo__ becomes foo.
> - if (Module.startswith("__") && Module.endswith("__"))
> - Module = Module.substr(2, Module.size() - 4);
> + StringRef ModuleName = Module->getName();
> + if (ModuleName.startswith("__") && ModuleName.endswith("__") &&
> + ModuleName.size() > 4) {
> + ModuleName = ModuleName.drop_front(2).drop_back(2);
> + Module = &S.PP.getIdentifierTable().get(ModuleName);
> + }
>
> SmallVector<unsigned, 8> OwnershipArgs;
> for (unsigned i = 1; i < AL.getNumArgs(); ++i) {
> @@ -1620,6 +1616,8 @@ static void handleOwnershipAttr(Sema &S,
> for (specific_attr_iterator<OwnershipAttr>
> i = D->specific_attr_begin<OwnershipAttr>(),
> e = D->specific_attr_end<OwnershipAttr>(); i != e; ++i) {
> + // FIXME: A returns attribute should conflict with any returns attribute
> + // with a different index too.
> if ((*i)->getOwnKind() != K && (*i)->args_end() !=
> std::find((*i)->args_begin(), (*i)->args_end(), Idx)) {
> S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible)
> @@ -1635,7 +1633,7 @@ static void handleOwnershipAttr(Sema &S,
> llvm::array_pod_sort(start, start + size);
>
> D->addAttr(::new (S.Context)
> - OwnershipAttr(AL.getLoc(), S.Context, K, Module, start, size,
> + OwnershipAttr(AL.getLoc(), S.Context, Module, start, size,
> AL.getAttributeSpellingListIndex()));
> }
>
> @@ -4698,10 +4696,7 @@ static void ProcessDeclAttribute(Sema &S
> case AttributeList::AT_NoCommon: handleNoCommonAttr (S, D, Attr); break;
> case AttributeList::AT_NonNull: handleNonNullAttr (S, D, Attr); break;
> case AttributeList::AT_Overloadable:handleOverloadableAttr(S, D, Attr); break;
> - case AttributeList::AT_ownership_returns:
> - case AttributeList::AT_ownership_takes:
> - case AttributeList::AT_ownership_holds:
> - handleOwnershipAttr (S, D, Attr); break;
> + case AttributeList::AT_Ownership: handleOwnershipAttr (S, D, Attr); break;
> case AttributeList::AT_Cold: handleColdAttr (S, D, Attr); break;
> case AttributeList::AT_Hot: handleHotAttr (S, D, Attr); break;
> case AttributeList::AT_Naked: handleNakedAttr (S, D, Attr); break;
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=195805&r1=195804&r2=195805&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Tue Nov 26 19:46:48 2013
> @@ -234,9 +234,9 @@ private:
> bool isAllocationFunction(const FunctionDecl *FD, ASTContext &C) const;
> bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const;
> ///@}
> - static ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
> - const CallExpr *CE,
> - const OwnershipAttr* Att);
> + ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
> + const CallExpr *CE,
> + const OwnershipAttr* Att) const;
> static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
> const Expr *SizeEx, SVal Init,
> ProgramStateRef State,
> @@ -729,10 +729,10 @@ void MallocChecker::checkPostObjCMessage
> C.addTransition(State);
> }
>
> -ProgramStateRef MallocChecker::MallocMemReturnsAttr(CheckerContext &C,
> - const CallExpr *CE,
> - const OwnershipAttr* Att) {
> - if (Att->getModule() != "malloc")
> +ProgramStateRef
> +MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE,
> + const OwnershipAttr *Att) const {
> + if (Att->getModule() != II_malloc)
> return 0;
>
> OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end();
> @@ -804,8 +804,8 @@ ProgramStateRef MallocChecker::MallocUpd
>
> ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
> const CallExpr *CE,
> - const OwnershipAttr* Att) const {
> - if (Att->getModule() != "malloc")
> + const OwnershipAttr *Att) const {
> + if (Att->getModule() != II_malloc)
> return 0;
>
> ProgramStateRef State = C.getState();
>
> Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=195805&r1=195804&r2=195805&view=diff
> ==============================================================================
> --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
> +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Tue Nov 26 19:46:48 2013
> @@ -1513,39 +1513,29 @@ void EmitClangAttrSpellingListIndex(Reco
> continue;
>
> std::vector<Record*> Spellings = R.getValueAsListOfDefs("Spellings");
> - // Each distinct spelling yields an attribute kind.
> - if (R.getValueAsBit("DistinctSpellings")) {
> - for (unsigned I = 0; I < Spellings.size(); ++ I) {
> - OS <<
> - " case AT_" << Spellings[I]->getValueAsString("Name") << ": \n"
> - " Index = " << I << ";\n"
> - " break;\n";
> - }
> - } else {
> - OS << " case AT_" << R.getName() << " : {\n";
> - for (unsigned I = 0; I < Spellings.size(); ++ I) {
> - SmallString<16> Namespace;
> - if (Spellings[I]->getValueAsString("Variety") == "CXX11")
> - Namespace = Spellings[I]->getValueAsString("Namespace");
> - else
> - Namespace = "";
> -
> - OS << " if (Name == \""
> - << Spellings[I]->getValueAsString("Name") << "\" && "
> - << "SyntaxUsed == "
> - << StringSwitch<unsigned>(Spellings[I]->getValueAsString("Variety"))
> - .Case("GNU", 0)
> - .Case("CXX11", 1)
> - .Case("Declspec", 2)
> - .Case("Keyword", 3)
> - .Default(0)
> - << " && Scope == \"" << Namespace << "\")\n"
> - << " return " << I << ";\n";
> - }
> -
> - OS << " break;\n";
> - OS << " }\n";
> + OS << " case AT_" << R.getName() << " : {\n";
> + for (unsigned I = 0; I < Spellings.size(); ++ I) {
> + SmallString<16> Namespace;
> + if (Spellings[I]->getValueAsString("Variety") == "CXX11")
> + Namespace = Spellings[I]->getValueAsString("Namespace");
> + else
> + Namespace = "";
> +
> + OS << " if (Name == \""
> + << Spellings[I]->getValueAsString("Name") << "\" && "
> + << "SyntaxUsed == "
> + << StringSwitch<unsigned>(Spellings[I]->getValueAsString("Variety"))
> + .Case("GNU", 0)
> + .Case("CXX11", 1)
> + .Case("Declspec", 2)
> + .Case("Keyword", 3)
> + .Default(0)
> + << " && Scope == \"" << Namespace << "\")\n"
> + << " return " << I << ";\n";
> }
> +
> + OS << " break;\n";
> + OS << " }\n";
> }
>
> OS << " }\n";
> @@ -1662,26 +1652,10 @@ static ParsedAttrMap getParsedAttrList(c
> for (std::vector<Record*>::iterator I = Attrs.begin(), E = Attrs.end();
> I != E; ++I) {
> Record &Attr = **I;
> -
> - bool SemaHandler = Attr.getValueAsBit("SemaHandler");
> - bool DistinctSpellings = Attr.getValueAsBit("DistinctSpellings");
> -
> - if (SemaHandler) {
> - if (DistinctSpellings) {
> - std::vector<Record*> Spellings = Attr.getValueAsListOfDefs("Spellings");
> -
> - for (std::vector<Record*>::const_iterator I = Spellings.begin(),
> - E = Spellings.end(); I != E; ++I) {
> - std::string AttrName = (*I)->getValueAsString("Name");
> -
> - StringRef Spelling = NormalizeAttrName(AttrName);
> - R.push_back(std::make_pair(Spelling.str(), &Attr));
> - }
> - } else {
> - StringRef AttrName = Attr.getName();
> - AttrName = NormalizeAttrName(AttrName);
> - R.push_back(std::make_pair(AttrName.str(), *I));
> - }
> + if (Attr.getValueAsBit("SemaHandler")) {
> + StringRef AttrName = Attr.getName();
> + AttrName = NormalizeAttrName(AttrName);
> + R.push_back(std::make_pair(AttrName.str(), *I));
> }
> }
> return R;
> @@ -1750,19 +1724,16 @@ void EmitClangAttrParsedAttrKinds(Record
> for (std::vector<Record*>::iterator I = Attrs.begin(), E = Attrs.end();
> I != E; ++I) {
> Record &Attr = **I;
> -
> +
> bool SemaHandler = Attr.getValueAsBit("SemaHandler");
> bool Ignored = Attr.getValueAsBit("Ignored");
> - bool DistinctSpellings = Attr.getValueAsBit("DistinctSpellings");
> if (SemaHandler || Ignored) {
> std::vector<Record*> Spellings = Attr.getValueAsListOfDefs("Spellings");
>
> + StringRef AttrName = NormalizeAttrName(StringRef(Attr.getName()));
> for (std::vector<Record*>::const_iterator I = Spellings.begin(),
> E = Spellings.end(); I != E; ++I) {
> std::string RawSpelling = (*I)->getValueAsString("Name");
> - StringRef AttrName = NormalizeAttrName(DistinctSpellings
> - ? StringRef(RawSpelling)
> - : StringRef(Attr.getName()));
>
> SmallString<64> Spelling;
> if ((*I)->getValueAsString("Variety") == "CXX11") {
>
>
> _______________________________________________
> 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