ajpaverd added a comment.
Thanks for the feedback @aaron.ballman and @dmajor!
================
Comment at: clang/include/clang/Basic/Attr.td:2914
+ let Spellings = [Declspec<"guard">];
+ let Subjects = SubjectList<[Function]>;
+ let Args = [EnumArgument<"Guard", "GuardArg", ["nocf"], ["nocf"]>];
----------------
aaron.ballman wrote:
> Should we also support ObjC methods? What about things like lambdas or blocks?
>
> (Note, these could all be handled in a follow-up patch, I just wasn't certain
> if this attribute was specific to functions or anything that executes code.)
Good point! At the moment this attribute is only documented for functions, so
I'll have to investigate the expected behaviour for lambdas, blocks, and ObjC
methods and provide a follow-up patch.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:4422-4427
+ if (FD->hasAttr<CFGuardAttr>() &&
+ FD->getAttr<CFGuardAttr>()->getGuard() == CFGuardAttr::GuardArg::nocf)
{
+ if (!CI->getCalledFunction()) {
+ Attrs = Attrs.addAttribute(
+ getLLVMContext(), llvm::AttributeList::FunctionIndex,
"guard_nocf");
+ }
----------------
aaron.ballman wrote:
> ```
> if (const auto *A = FD->getAttr<CFGuardAttr>()) {
> if (A->getGuard() == CFGuardAttr::GuardArg::nocf &&
> !CI->getCalledFunction())
> Attrs = ...
> }
> ```
> (This way you don't have to call `hasAttr<>` followed by `getAttr<>`.)
That's much nicer - thanks!
================
Comment at: clang/test/Sema/attr-guard_nocf.cpp:1
+// RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11
-fsyntax-only %s
+
----------------
aaron.ballman wrote:
> Since this test file is identical to the C test, I would remove the .cpp file
> and add its RUN line to the .c test instead. You can do this with:
> ```
> // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11
> -fsyntax-only -x c++ %s
> ```
That saves a lot of repetition - thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72167/new/
https://reviews.llvm.org/D72167
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
