r303712 - Enhance the 'diagnose_if' attribute so that we can apply it for ObjC methods and properties as well
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed May 24 10:12:01 PDT 2017
On Wed, May 24, 2017 at 1:05 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>
>> On May 24, 2017, at 8:59 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Tue, May 23, 2017 at 8:46 PM, Argyrios Kyrtzidis via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>>> Author: akirtzidis
>>> Date: Tue May 23 19:46:27 2017
>>> New Revision: 303712
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=303712&view=rev
>>> Log:
>>> Enhance the 'diagnose_if' attribute so that we can apply it for ObjC methods and properties as well
>>>
>>> This is an initial commit to allow using it with constant expressions, a follow-up commit will enable full support for it in ObjC methods.
>>>
>>> Added:
>>> cfe/trunk/test/SemaObjC/diagnose_if.m
>>> Modified:
>>> cfe/trunk/include/clang/Basic/Attr.td
>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> cfe/trunk/include/clang/Sema/AttributeList.h
>>> cfe/trunk/include/clang/Sema/Sema.h
>>> cfe/trunk/lib/Lex/PPMacroExpansion.cpp
>>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>> cfe/trunk/lib/Sema/SemaExpr.cpp
>>> cfe/trunk/lib/Sema/SemaOverload.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=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/Attr.td (original)
>>> +++ cfe/trunk/include/clang/Basic/Attr.td Tue May 23 19:46:27 2017
>>> @@ -149,6 +149,9 @@ class ExprArgument<string name, bit opt
>>> class FunctionArgument<string name, bit opt = 0, bit fake = 0> : Argument<name,
>>> opt,
>>> fake>;
>>> +class NamedArgument<string name, bit opt = 0, bit fake = 0> : Argument<name,
>>> + opt,
>>> + fake>;
>>> class TypeArgument<string name, bit opt = 0> : Argument<name, opt>;
>>> class UnsignedArgument<string name, bit opt = 0> : Argument<name, opt>;
>>> class VariadicUnsignedArgument<string name> : Argument<name, 1>;
>>> @@ -1819,14 +1822,14 @@ def Unavailable : InheritableAttr {
>>>
>>> def DiagnoseIf : InheritableAttr {
>>> let Spellings = [GNU<"diagnose_if">];
>>> - let Subjects = SubjectList<[Function]>;
>>> + let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
>>> let Args = [ExprArgument<"Cond">, StringArgument<"Message">,
>>> EnumArgument<"DiagnosticType",
>>> "DiagnosticType",
>>> ["error", "warning"],
>>> ["DT_Error", "DT_Warning"]>,
>>> BoolArgument<"ArgDependent", 0, /*fake*/ 1>,
>>> - FunctionArgument<"Parent", 0, /*fake*/ 1>];
>>> + NamedArgument<"Parent", 0, /*fake*/ 1>];
>>> let DuplicatesAllowedWhileMerging = 1;
>>> let LateParsed = 1;
>>> let AdditionalMembers = [{
>>>
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 23 19:46:27 2017
>>> @@ -2771,6 +2771,7 @@ def warn_attribute_wrong_decl_type : War
>>> "|types and namespaces"
>>> "|Objective-C interfaces"
>>> "|methods and properties"
>>> + "|functions, methods and properties"
>>
>> functions, methods, and properties (inserting the Oxford comma).
>
> Ok.
>
>>
>>> "|struct or union"
>>> "|struct, union or class"
>>> "|types"
>>>
>>> Modified: cfe/trunk/include/clang/Sema/AttributeList.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Sema/AttributeList.h (original)
>>> +++ cfe/trunk/include/clang/Sema/AttributeList.h Tue May 23 19:46:27 2017
>>> @@ -915,6 +915,7 @@ enum AttributeDeclKind {
>>> ExpectedTypeOrNamespace,
>>> ExpectedObjectiveCInterface,
>>> ExpectedMethodOrProperty,
>>> + ExpectedFunctionOrMethodOrProperty,
>>> ExpectedStructOrUnion,
>>> ExpectedStructOrUnionOrClass,
>>> ExpectedType,
>>>
>>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>>> +++ cfe/trunk/include/clang/Sema/Sema.h Tue May 23 19:46:27 2017
>>> @@ -2727,7 +2727,7 @@ public:
>>> /// of a function.
>>> ///
>>> /// Returns true if any errors were emitted.
>>> - bool diagnoseArgIndependentDiagnoseIfAttrs(const FunctionDecl *Function,
>>> + bool diagnoseArgIndependentDiagnoseIfAttrs(const NamedDecl *ND,
>>> SourceLocation Loc);
>>>
>>> /// Returns whether the given function's address can be taken or not,
>>>
>>> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
>>> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Tue May 23 19:46:27 2017
>>> @@ -1166,6 +1166,7 @@ static bool HasFeature(const Preprocesso
>>> .Case("objc_generics", LangOpts.ObjC2)
>>> .Case("objc_generics_variance", LangOpts.ObjC2)
>>> .Case("objc_class_property", LangOpts.ObjC2)
>>> + .Case("objc_diagnose_if_attr", LangOpts.ObjC2)
>>> // C11 features
>>> .Case("c_alignas", LangOpts.C11)
>>> .Case("c_alignof", LangOpts.C11)
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue May 23 19:46:27 2017
>>> @@ -949,7 +949,7 @@ static bool checkFunctionConditionAttr(S
>>> Msg = "<no message provided>";
>>>
>>> SmallVector<PartialDiagnosticAt, 8> Diags;
>>> - if (!Cond->isValueDependent() &&
>>> + if (isa<FunctionDecl>(D) && !Cond->isValueDependent() &&
>>> !Expr::isPotentialConstantExprUnevaluated(Cond, cast<FunctionDecl>(D),
>>> Diags)) {
>>> S.Diag(Attr.getLoc(), diag::err_attr_cond_never_constant_expr)
>>> @@ -1037,10 +1037,11 @@ static void handleDiagnoseIfAttr(Sema &S
>>> return;
>>> }
>>>
>>> - auto *FD = cast<FunctionDecl>(D);
>>> - bool ArgDependent = ArgumentDependenceChecker(FD).referencesArgs(Cond);
>>> + bool ArgDependent = false;
>>> + if (auto *FD = dyn_cast<FunctionDecl>(D))
>>
>> Please use const auto *.
>>
>>> + ArgDependent = ArgumentDependenceChecker(FD).referencesArgs(Cond);
>>> D->addAttr(::new (S.Context) DiagnoseIfAttr(
>>> - Attr.getRange(), S.Context, Cond, Msg, DiagType, ArgDependent, FD,
>>> + Attr.getRange(), S.Context, Cond, Msg, DiagType, ArgDependent, cast<NamedDecl>(D),
>>> Attr.getAttributeSpellingListIndex()));
>>> }
>>>
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue May 23 19:46:27 2017
>>> @@ -366,8 +366,19 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *
>>>
>>> if (getLangOpts().CUDA && !CheckCUDACall(Loc, FD))
>>> return true;
>>> + }
>>>
>>> - if (diagnoseArgIndependentDiagnoseIfAttrs(FD, Loc))
>>> + auto getReferencedObjCProp = [](const NamedDecl *D) ->
>>> + const ObjCPropertyDecl * {
>>> + if (auto *MD = dyn_cast<ObjCMethodDecl>(D))
>>
>> Please use const auto *.
>
> What is the rationale for this, the casted object is ‘const’ already so it’s unnecessary, and I don’t see a mention in coding conventions that we should be using ‘const’ explicitly along with auto.
That is our usual advice -- why should the reader have to guess
whether the pointee type is const or not? It is in the coding
standard, but could stand to be called out more explicitly. See the
last example in
http://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto.
~Aaron
>>
>>> + return MD->findPropertyDecl();
>>> + return nullptr;
>>> + };
>>> + if (auto *ObjCPDecl = getReferencedObjCProp(D)) {
>>
>> Please do not use auto here (the type is not spelled out in the
>> initialization). Also, please mark it explicitly as being a const
>> pointer.
>
> Ok.
>
>>
>>> + if (diagnoseArgIndependentDiagnoseIfAttrs(ObjCPDecl, Loc))
>>> + return true;
>>> + } else {
>>> + if (diagnoseArgIndependentDiagnoseIfAttrs(D, Loc))
>>
>> Please bump the if up to be part of the else clause.
>
> Ok.
>
>>
>> ~Aaron
>>
>>> return true;
>>> }
>>>
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue May 23 19:46:27 2017
>>> @@ -6242,11 +6242,11 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
>>> }
>>>
>>> template <typename CheckFn>
>>> -static bool diagnoseDiagnoseIfAttrsWith(Sema &S, const FunctionDecl *FD,
>>> +static bool diagnoseDiagnoseIfAttrsWith(Sema &S, const NamedDecl *ND,
>>> bool ArgDependent, SourceLocation Loc,
>>> CheckFn &&IsSuccessful) {
>>> SmallVector<const DiagnoseIfAttr *, 8> Attrs;
>>> - for (const auto *DIA : FD->specific_attrs<DiagnoseIfAttr>()) {
>>> + for (const auto *DIA : ND->specific_attrs<DiagnoseIfAttr>()) {
>>> if (ArgDependent == DIA->getArgDependent())
>>> Attrs.push_back(DIA);
>>> }
>>> @@ -6293,16 +6293,16 @@ bool Sema::diagnoseArgDependentDiagnoseI
>>> // EvaluateWithSubstitution only cares about the position of each
>>> // argument in the arg list, not the ParmVarDecl* it maps to.
>>> if (!DIA->getCond()->EvaluateWithSubstitution(
>>> - Result, Context, DIA->getParent(), Args, ThisArg))
>>> + Result, Context, cast<FunctionDecl>(DIA->getParent()), Args, ThisArg))
>>> return false;
>>> return Result.isInt() && Result.getInt().getBoolValue();
>>> });
>>> }
>>>
>>> -bool Sema::diagnoseArgIndependentDiagnoseIfAttrs(const FunctionDecl *Function,
>>> +bool Sema::diagnoseArgIndependentDiagnoseIfAttrs(const NamedDecl *ND,
>>> SourceLocation Loc) {
>>> return diagnoseDiagnoseIfAttrsWith(
>>> - *this, Function, /*ArgDependent=*/false, Loc,
>>> + *this, ND, /*ArgDependent=*/false, Loc,
>>> [&](const DiagnoseIfAttr *DIA) {
>>> bool Result;
>>> return DIA->getCond()->EvaluateAsBooleanCondition(Result, Context) &&
>>>
>>> Added: cfe/trunk/test/SemaObjC/diagnose_if.m
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/diagnose_if.m?rev=303712&view=auto
>>> ==============================================================================
>>> --- cfe/trunk/test/SemaObjC/diagnose_if.m (added)
>>> +++ cfe/trunk/test/SemaObjC/diagnose_if.m Tue May 23 19:46:27 2017
>>> @@ -0,0 +1,16 @@
>>> +// RUN: %clang_cc1 %s -verify -fno-builtin
>>> +
>>> +_Static_assert(__has_feature(objc_diagnose_if_attr), "feature check failed?");
>>> +
>>> +#define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__)))
>>> +
>>> + at interface I
>>> +-(void)meth _diagnose_if(1, "don't use this", "warning"); // expected-note 1{{from 'diagnose_if'}}
>>> + at property (assign) id prop _diagnose_if(1, "don't use this", "warning"); // expected-note 2{{from 'diagnose_if'}}
>>> + at end
>>> +
>>> +void test(I *i) {
>>> + [i meth]; // expected-warning {{don't use this}}
>>> + id o1 = i.prop; // expected-warning {{don't use this}}
>>> + id o2 = [i prop]; // expected-warning {{don't use this}}
>>> +}
>>>
>>> Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=303712&r1=303711&r2=303712&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
>>> +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Tue May 23 19:46:27 2017
>>> @@ -312,7 +312,7 @@ namespace {
>>> }
>>>
>>> void writeDump(raw_ostream &OS) const override {
>>> - if (type == "FunctionDecl *") {
>>> + if (type == "FunctionDecl *" || type == "NamedDecl *") {
>>> OS << " OS << \" \";\n";
>>> OS << " dumpBareDeclRef(SA->get" << getUpperName() << "());\n";
>>> } else if (type == "IdentifierInfo *") {
>>> @@ -1181,6 +1181,8 @@ createArgument(const Record &Arg, String
>>> Ptr = llvm::make_unique<ExprArgument>(Arg, Attr);
>>> else if (ArgName == "FunctionArgument")
>>> Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "FunctionDecl *");
>>> + else if (ArgName == "NamedArgument")
>>> + Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "NamedDecl *");
>>> else if (ArgName == "IdentifierArgument")
>>> Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "IdentifierInfo *");
>>> else if (ArgName == "DefaultBoolArgument")
>>> @@ -3103,6 +3105,8 @@ static std::string CalculateDiagnostic(c
>>> " : ExpectedVariableOrFunction))";
>>>
>>> case ObjCMethod | ObjCProp: return "ExpectedMethodOrProperty";
>>> + case Func | ObjCMethod | ObjCProp:
>>> + return "ExpectedFunctionOrMethodOrProperty";
>>> case ObjCProtocol | ObjCInterface:
>>> return "ExpectedObjectiveCInterfaceOrProtocol";
>>> case Field | Var: return "ExpectedFieldOrGlobalVar";
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list