On Oct 31, 2012, at 11:55 AM, Quentin Colombet <qcolom...@apple.com> wrote:
> > On Oct 30, 2012, at 1:40 PM, Dmitri Gribenko <griboz...@gmail.com> wrote: > >> On Tue, Oct 30, 2012 at 10:29 PM, Quentin Colombet <qcolom...@apple.com> >> wrote: >>> >>> On Oct 30, 2012, at 12:36 PM, Dmitri Gribenko <griboz...@gmail.com> wrote: >>> >>>> Hi Quentin, >>>> >>>> On Tue, Oct 30, 2012 at 9:13 PM, Quentin Colombet <qcolom...@apple.com> >>>> wrote: >>>>> Following commits r167020 and r167021, which define MinSize attribute for >>>>> functions and set it when Oz optimization level is used, I propose this >>>>> patch to be able to use this attribute (minsize) directly in the front >>>>> end. >>>> >>>> + if (!isa<FunctionDecl>(D)) { >>>> + S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) >>>> + << Attr.getName() << ExpectedFunction; >>>> + return; >>>> + } >>>> >>>> Why not reject this with an error? This is a new attribute, there is >>>> no legacy code misusing it, nor other compilers tolerating such >>>> misuse. >>> >>> >>> Thank Dimitri. >>> You are right. I turned it into an error. New patch attached. >> >> It would be good to have tests in test/Sema for cases we reject. >> >> Sorry for not saying this right away, I just thought about it. > > The new patch with the added test case in test/Sema +static void handleMinSizeAttr(Sema &S, Decl *D, const AttributeList &Attr) { + // Check the attribute arguments. + if (!checkAttributeNumArgs(S, Attr, 0)) + return; + + if (!isa<FunctionDecl>(D)) { + S.Diag(Attr.getLoc(), diag::err_attribute_wrong_decl_type) + << Attr.getName() << ExpectedFunction; + return; + } + + D->addAttr(::new (S.Context) MinSizeAttr(Attr.getRange(), S.Context)); +} Should this also work for Objective-C methods, e.g., ObjCMethodDecls? Also, please add a test involving C++ function templates, to be sure that the attribute gets propagated to the instantiation correctly. - Doug _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits