tambre marked 4 inline comments as done.
tambre added a comment.
Thanks for the reviews and design pointers, John!
There are still a few tests failing, but I'd rather not dive in before the
general approach is considered acceptable.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:8880
+ }
+ }
+
----------------
rjmccall wrote:
> Hmm. I'm concerned about not doing any sort of semantic compatibility check
> here before we assign the function special semantics. Have we really never
> done those in C++?
>
> If not, I wonder if we can reasonably make an implicit declaration but just
> make it hidden from lookup.
Currently there's no semantic compatibility checking for builtin
redeclarations. There is for initial declarations though.
I've added this checking by splitting the actual builtin declaration creation
off from `LazilyCreateBuiltin` into `CreateBuiltin` and checking if the current
declaration is compatible with what the builtin's would be.
This results in stronger typechecking than before for builtin declarations, so
some incompatible declarations are no longer marked as builtin. See
`cxx1z-noexcept-function-type.cpp` for an example.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:6047
OverloadDecl->setParams(Params);
+ Sema->mergeDeclAttributes(OverloadDecl, FDecl);
return OverloadDecl;
----------------
`rewriteBuiltinFunctionDecl()` creates a new function declaration and happened
to disregard the builtin's attributes. This caused `BuiltinAttr` to go missing
in a bunch of OpenCL tests, resulting in failures.
================
Comment at: clang/test/Sema/warn-fortify-source.c:20
void *memcpy(void *dst, const void *src, size_t c);
static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void
*src, size_t c) __attribute__((overloadable)) __asm__("merp");
static void *memcpy(void *const dst __attribute__((pass_object_size(1))),
const void *src, size_t c) __attribute__((overloadable)) {
----------------
Not quite sure what to do here. These were previously recognized as builtins
due to their name despite being incompatible and thus had fortify checking
similar to the real `memcpy`.
Maybe:
1. Introduce a generic version of `ArmBuiltinAliasAttr`.
2. Something like `FormatAttr`.
================
Comment at: clang/test/SemaCXX/cxx11-compat.cpp:35
+#define PRIuS "zu"
int printf(const char *, ...);
typedef __typeof(sizeof(int)) size_t;
----------------
For some reason this `printf` didn't get format checking before. Same for
`SemaCXX/warn-unused-local-typedef.cpp`.
================
Comment at: clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp:120
extern "C" int strcmp(const char *, const char *);
- extern "C" int strncmp(const char *, const char *, decltype(sizeof(0)))
noexcept;
+ extern "C" int strncmp(const char *, const char *, decltype(sizeof(0)));
----------------
This doesn't work anymore since we now properly check builtin declaration
compatibility and since C++17 noexcept is part of the function type but
builtins aren't noexcept.
Thoughts?
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
