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::ActOnStartOfFunctionDef is called.
----------------
aaron.ballman wrote:
> handleNoBuiltin -> handleNoBuiltinAttr
I am not convinced that this is a bug -- the function declaration does not
become a definition until the parser reaches the definition.
In any case, I don't think the predicate you want is "is this declaration a
definition?". Rather, I think what you want is, "does this declaration
introduce an explicit function body?". In particular, we should not permit uses
of this attribute on defaulted or deleted functions, nor on functions that have
a definition by virtue of using `__attribute__((alias))`. So it probably should
be a syntactic check on the form of the declarator (that is, the check you're
perrforming here), and the check should probably be
`D.getFunctionDefinitionKind() == FDK_Definition`. (A custom diagnostic for
using the attribute on a defaulted or deleted function would be nice too, since
the existing diagnostic text isn't really accurate in those cases.)
================
Comment at: clang/test/Sema/no-builtin.c:7
+
+void no_builtin_no_argument() __attribute__((no_builtin)) {}
+// expected-error@-1 {{'no_builtin' attribute takes at least 1 argument}}
----------------
I find this surprising. I would expect this to work, and to mean the same as
`-fno-builtin` for the function (that is, same as the `*` form).
I think might be a better design to remove the magical `"*"` form entirely, and
use an empty argument list to mean "all builtins". Have you considered that?
(That would better mirror how `-fno-builtin` vs `-fno-builtin-foo` works.)
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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
