[PATCH] D68028: [clang] Add no_builtin attribute

This revision was automatically updated to reflect the committed changes. Closed by commit rGbd8791610948: [clang] Add no_builtin attribute (authored by gchatelet). Changed prior to commit: https://reviews.llvm.org/D68028?vs=225618&id=226687#toc Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet added a comment. In D68028#1723568 , @aaron.ballman wrote: > LGTM! Thx a lot for bearing with me and for your valuable comments ! Really appreciate it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet marked an inline comment as done. gchatelet added a comment. LGTY @aaron.ballman ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 ___ cfe-commits mai

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 225618. gchatelet added a comment. - Fix documentation and Warning category Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/include/clang/AST/Decl.h

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet marked 4 inline comments as done. gchatelet added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3609 + "'%0' is not a valid builtin name for %1">, + InGroup; // Which group should it be in? +def err_attribute_no_builtin_wildcard_or_buil

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4407 +It accepts one or more strings corresponding to the name of the builtin +(e.g. "memcpy", "memset") or "*" which disables all builtins at once. + This mention of `*` is no

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 225593. gchatelet added a comment. - Call sites to virtual functions are not annotated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/include/clang/AS

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 225578. gchatelet added a comment. - Reverting whole file formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/include/clang/AST/Decl.h clang/i

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 225576. gchatelet marked an inline comment as done. gchatelet added a comment. - Improve tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/include/

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 225575. gchatelet marked 3 inline comments as done. gchatelet added a comment. - Change NoBuiltin from InheritableAttr to Attr - Improve tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3416 + +def NoBuiltin : InheritableAttr { + let Spellings = [Clang<"no_builtin">]; I think we do not want this to be an inheritable attribute, but just `Attr` instead, because th

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 225435. gchatelet marked 13 inline comments as done. gchatelet added a comment. Herald added subscribers: s.egerton, simoncook, aheejin, dschuff. - Address comments - Fix missing rename - Address comments - Add warning category Repository: rG LLVM Github

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3600 "attribute">; +def err_attribute_no_builtin_invalid_builtin_name : Error< + "'%0' is not a valid builtin name for %1">; aaron.ballman wrote: > I think this shou

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:9508-9510 + // FIXME: We should really be doing this in SemaDeclAttr.cpp::handleNoBuiltin + // but there is a bug with FunctionDecl::isThisDeclarationADefinition() which + // always returns false befo

[PATCH] D68028: [clang] Add no_builtin attribute

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:9508-9510 + // FIXME: We should really be doing this in SemaDeclAttr.cpp::handleNoBuiltin + // but there is a bug with FunctionDecl::isThisDeclarationADefinition() which + // always returns false before Sema

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:9508-9510 + // FIXME: We should really be doing this in SemaDeclAttr.cpp::handleNoBuiltin + // but there is a bug with FunctionDecl::isThisDeclarationADefinition() which + // always returns false befo

[PATCH] D68028: [clang] Add no_builtin attribute

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:9508-9510 + // FIXME: We should really be doing this in SemaDeclAttr.cpp::handleNoBuiltin + // but there is a bug with FunctionDecl::isThisDeclarationADefinition() which + // always returns false before Sema

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added a comment. In D68028#1707931 , @gchatelet wrote: > A gentle ping @aaron.ballman Sorry for the delay, I was traveling for last week. Comment at: clang/include/clang/Basic/AttrDocs.td:4402 +The ``__attribute__((no_bu

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet added a comment. A gentle ping @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 224286. gchatelet added a comment. - Fix missing rename Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/include/clang/Basic/Attr.td clang/include/cla

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 224285. gchatelet marked 2 inline comments as done. gchatelet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/inclu

[PATCH] D68028: [clang] Add no_builtin attribute

courbet added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3418 + let Spellings = [Clang<"no_builtin">]; + let Args = [VariadicStringArgument<"FunctionNames">]; + let Subjects = SubjectList<[Function]>; [nit] Should this be `BuiltinNames` ? b

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet added a comment. It should be ready to submit, @aaron.ballman @courbet can you take a final look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 ___

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 224271. gchatelet added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/Att

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 224085. gchatelet added a comment. - Remove leftovers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/include/clang/Basic/Attr.td clang/include/clang

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 224082. gchatelet added a comment. - Address aaron ballman comments - Checks function name validity and errors when passed 0 argument. - Update documentation and rebase - Rewrote mergeNoBuiltinAttr - Address comments Repository: rG LLVM Github Monorepo

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet marked an inline comment as done. gchatelet added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1098-1099 + + if (D->hasAttr()) +D->dropAttr(); + gchatelet wrote: > aaron.ballman wrote: > > gchatelet wrote: > > > gchatelet wrote: > >

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet marked 2 inline comments as done. gchatelet added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1098-1099 + + if (D->hasAttr()) +D->dropAttr(); + aaron.ballman wrote: > gchatelet wrote: > > gchatelet wrote: > > > aaron.ballman wrote:

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added a subscriber: rsmith. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1072 +NoBuiltinAttr * +Sema::mergeNoBuiltinAttr(Sema &S, Decl *D, const AttributeCommonInfo &CI, + llvm::ArrayRef FunctionNames) { --

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet marked 3 inline comments as done. gchatelet added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1098-1099 + + if (D->hasAttr()) +D->dropAttr(); + gchatelet wrote: > aaron.ballman wrote: > > Just making sure I understand the semantics

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet marked 7 inline comments as done. gchatelet added a comment. thx @aaron.ballman , I'm waiting for your reply before uploading the new patch (some of my comments won't have the accompanying code update sorry) Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1072 +NoBuiltin

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1092 + // Wildcard is a super set of all builtins, we keep only this one. + if (FunctionNames.count(Wildcard) > 0) { +FunctionNames.clear();

[PATCH] D68028: [clang] Add no_builtin attribute

tejohnson added a comment. In D68028#1685912 , @gchatelet wrote: > @tejohnson I believe this is the missing part for D67923 > . Thanks, yep I will take a closer look at the patch today. > I'm unsure if we still need the

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 222163. gchatelet added a comment. - Update documentation and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/include/clang/Basic/Attr.td clan

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet added a comment. @tejohnson I believe this is the missing part for D67923 . I'm unsure if we still need the `BitVector` at all in the `TLI` since it could be a simple attribute lookup on the function. Do you see any problematic interactions with the i

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 221939. gchatelet added a comment. - Checks function name validity and errors when passed 0 argument. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files: clang/i

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet added a comment. @aaron.ballman thx a lot for the review. I really appreciate it, especially because I'm not too familiar with this part of the codebase. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1092 + // Wildcard is a super set of all builtins, we keep only this

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet updated this revision to Diff 221924. gchatelet marked 11 inline comments as done. gchatelet added a comment. - Address aaron ballman comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68028/new/ https://reviews.llvm.org/D68028 Files

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. Thank you for working on this! It looks like you're missing all of the sema tests that check that the attribute only appertains to functions, accepts the proper kind of arguments, etc. Comment at:

[PATCH] D68028: [clang] Add no_builtin attribute

gchatelet created this revision. gchatelet added reviewers: tejohnson, courbet, theraven, t.p.northover, jdoerfert. Herald added subscribers: cfe-commits, mgrang. Herald added a project: clang. This is a follow up on https://reviews.llvm.org/D61634 This patch is simpler and only adds the no_built