[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbc5f2d12cadc: [clang] diagnose_as_builtin attribute for Fortify diagnosing like builtins. (authored by mbenfield). Repository: rG LLVM Github

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 393942. mbenfield added a comment. Herald added a subscriber: jdoerfert. Fix test clang/test/Misc/pragma-attribute-supported-attributes-list.test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

aaron.ballman accepted this revision. aaron.ballman added a comment. In D112024#3180347 , @mbenfield wrote: > @aaron.ballman Look to you like this is ready to land? > > EDIT: oops sorry, missed your latest comments, will address them shortly. Thanks for

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 393226. mbenfield added a comment. Remove late parsing. Switch dyn_cast to cast. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 Files:

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield added a comment. @aaron.ballman Look to you like this is ready to land? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 ___ cfe-commits mailing list

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 392830. mbenfield added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 Files: clang/include/clang/Basic/Attr.td

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3873 + let Subjects = SubjectList<[Function]>; + let LateParsed = 1; + let Documentation = [DiagnoseAsBuiltinDocs]; Why does this need to be late parsed? I don't think the

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 391076. mbenfield marked 8 inline comments as done. mbenfield added a comment. Replace DeclFD->getName() with just DeclFD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 391067. mbenfield added a comment. Revert spurious whitespace change. assert(D) No else after return. Allow attribute to be applied to static member functions and test this. Document that it can't be applied to a non-static member function.

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5956 +given. In addition, the order in which arguments should be applied must also +be given. + We should mention that the attribute cannot be applied to a member function

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 389355. mbenfield added a comment. Error on member function. Test case for this. Test case for attribute applied to a non-function declaration. Handle variadic functions. Document this. Test case for variadic functions. Remove const from some local

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5973 +``mymemset(n, c, s)`` will diagnose overflows as if it were the call +``__builtin_memset(s, c, n)``; +}]; One thing that's not quite clear from the docs is how to handle

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 387389. mbenfield added a comment. s/requires/takes in diagnostic message. Test case with a non-template overloaded function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

george.burgess.iv added inline comments. Comment at: clang/test/Sema/attr-diagnose-as-builtin.c:62 + +#ifdef __cplusplus +template mbenfield wrote: > george.burgess.iv wrote: > > nit: can we also add a non-templated overload check in here? > > > > if the diag

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield added inline comments. Comment at: clang/test/Sema/attr-diagnose-as-builtin.c:62 + +#ifdef __cplusplus +template george.burgess.iv wrote: > nit: can we also add a non-templated overload check in here? > > if the diag isn't beautiful, that's fine IMO.

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. LGTM % 2 nits. Please feel free to commit after it LGT @aaron.ballman too. :) Thanks again! Comment at:

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 386623. mbenfield added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 Files: clang/include/clang/Basic/Attr.td

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield added a comment. Would @george.burgess.iv and/or @aaron.ballman mind taking another look at the latest revision? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 385551. mbenfield added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112024/new/ https://reviews.llvm.org/D112024 Files: clang/include/clang/Basic/Attr.td

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield marked 8 inline comments as done. mbenfield added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3822 +def DiagnoseAs : InheritableAttr { + let Spellings = [Clang<"diagnose_as">]; + let Args = [ExprArgument<"Function">,

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield updated this revision to Diff 384451. mbenfield added a comment. Renamed to diagnose_as_builtin (since two people suggested that name). Let me know if a name mentioning fortify is preferred. Validation in handleDiagnoseAsBuiltinAttr: - first argument is a builtin function - number of

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

serge-sans-paille added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3822 +def DiagnoseAs : InheritableAttr { + let Spellings = [Clang<"diagnose_as">]; + let Args = [ExprArgument<"Function">, aaron.ballman wrote: > george.burgess.iv wrote: >

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3822 +def DiagnoseAs : InheritableAttr { + let Spellings = [Clang<"diagnose_as">]; + let Args = [ExprArgument<"Function">, george.burgess.iv wrote: > purely subjective nit:

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

george.burgess.iv added a comment. Thanks for this! The idea LGTM, and sounds like a solid way for us to do better about diagnosing FORTIFY'ed calls in Clang. I have a handful of mostly nits/questions for you :) Comment at: clang/include/clang/Basic/Attr.td:3822 +def

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

mbenfield created this revision. mbenfield added a reviewer: george.burgess.iv. Herald added a reviewer: aaron.ballman. mbenfield requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo