aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM with a few minor changes to be made.
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1364
+ assert(Ptr && "Cannot create argument.");
+
----------------
OikawaKirie wrote:
> Just as what has been suggested by @dexonsmith in D91844, I add the assertion
> here. However, as a recursive function it is (line 1359), I still concern
> whether the assertion here will lead to a crash.
>
> What do you think?
The intent with this function is that it never returns `nullptr` unless the
programmer messed up their call of the function (passed in the wrong kind of
`Record` kind of problem). All of the callers assume this returns non-null, so
I think it's reasonable to assert that the value is non-null by this point.
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1364
+ assert(Ptr && "Cannot create argument.");
+
----------------
aaron.ballman wrote:
> OikawaKirie wrote:
> > Just as what has been suggested by @dexonsmith in D91844, I add the
> > assertion here. However, as a recursive function it is (line 1359), I still
> > concern whether the assertion here will lead to a crash.
> >
> > What do you think?
> The intent with this function is that it never returns `nullptr` unless the
> programmer messed up their call of the function (passed in the wrong kind of
> `Record` kind of problem). All of the callers assume this returns non-null,
> so I think it's reasonable to assert that the value is non-null by this point.
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1366-1370
if (Ptr && Arg.getValueAsBit("Optional"))
Ptr->setOptional(true);
if (Ptr && Arg.getValueAsBit("Fake"))
Ptr->setFake(true);
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97251/new/
https://reviews.llvm.org/D97251
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
