tambre marked an inline comment as done.
tambre added inline comments.
================
Comment at: clang/lib/AST/Decl.cpp:3169-3175
} else {
- if (!getIdentifier())
+ const auto *Attr = getAttr<BuiltinAttr>();
+
+ if (!Attr)
return 0;
+ BuiltinID = Attr->getID();
----------------
aaron.ballman wrote:
> I think this is a bit more clear:
> ```
> } else if (const auto *A = getAttr<BuiltinAttr>()) {
> BuiltinID = A->getID();
> }
> ```
> and initialize `BuiltinID` to zero above.
Done.
================
Comment at: clang/test/CodeGen/callback_pthread_create.c:17
+// FIXME: How to do builtin handling for this?
int pthread_create(pthread_t *, const pthread_attr_t *,
----------------
As many others prior to this patch, `pthread_create` was recognized as a
builtin due to its name and thus had attributes applied.
Unlike others however, `pthread_create` is the only builtin in `Builtins.def`
that doesn't have its arguments specified. Doing that would require
implementing support for function pointers in the builtin database and adding
yet another special case for `pthread_t` and `pthread_attr_t`.
That'd be quite a bit of work, which I'm not interested in doing.
How about simply removing the hack that is the `pthread_create` builtin entry
and disabling/removing this test?
================
Comment at: clang/test/Sema/implicit-builtin-decl.c:64
-struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration
of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header <setjmp.h>.}}
----------------
aaron.ballman wrote:
> It looks like we're losing test coverage with this change?
Indeed. I've reverted this change and changed the test to instead not test for
it being recognized as a builtin, since it shouldn't be.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77491/new/
https://reviews.llvm.org/D77491
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
