================
@@ -3328,3 +3300,63 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
}
}
}
+
+void clang::checkUnsafeBufferUsage(const Decl *D,
+ UnsafeBufferUsageHandler &Handler,
+ bool EmitSuggestions) {
+#ifndef NDEBUG
+ Handler.clearDebugNotes();
+#endif
+
+ assert(D);
+
+ SmallVector<Stmt *> Stmts;
+
+ // We do not want to visit a Lambda expression defined inside a method
+ // independently. Instead, it should be visited along with the outer method.
+ // FIXME: do we want to do the same thing for `BlockDecl`s?
+ if (const auto *fd = dyn_cast<CXXMethodDecl>(D)) {
+ if (fd->getParent()->isLambda() && fd->getParent()->isLocalClass())
+ return;
+ }
+
+ // Do not emit fixit suggestions for functions declared in an
+ // extern "C" block.
+ if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+ for (FunctionDecl *FReDecl : FD->redecls()) {
+ if (FReDecl->isExternC()) {
+ EmitSuggestions = false;
+ break;
+ }
+ }
+
+ Stmts.push_back(FD->getBody());
+
+ if (const auto *ID = dyn_cast<CXXConstructorDecl>(D)) {
+ for (const CXXCtorInitializer *CI : ID->inits()) {
+ Stmts.push_back(CI->getInit());
+ }
+ }
+ }
+
+ if (const auto *FD = dyn_cast<FieldDecl>(D)) {
----------------
haoNoQ wrote:
> AFAICT we are required to visit FieldDecl to handle the 0-ctor class use case.
I really want us to momentarily stop being preoccupied with "can" and focus on
"should". This isn't a technical problem, this is a UI/UX problem.
There's no constructor. There's no binary code for constructor. The unsafe
function is not invoked from any binary code generated from your example. It's
the responsibility of the code that aggregate-initializes your aggregate to
generate the call to the default initializer, so naturally it doesn't show up
just yet.
The class is safe to use whenever the field is initialized explicitly. If they
remove the field initializer, that'll break backwards compatibility. Maybe it's
ok to keep the class as-is and use the attribute only as a reminder to the
callers to call a better constructor.
Ultimately I think you're right and in most of such cases the best solution is
to change the class to use a different, safe default initializer for the field.
Which presumably exists, given that the attribute was placed on the currently
used constructor. I think it's true that more users would complaint about
"false negative" in the aggregate definition, or an awkwardly placed /
hard-to-read warning at the curly braces (especially if the unsafe initializer
is layered into several other layers of aggregates) than complain about false
positive under "the class is intended to be safe as long as the field
initializer is passed explicitly".
But this is something we should occasionally think about, and treat this as a
very very special cornercase. Which doesn't even necessarily require an
immediate solution - given that all unsafe code will eventually be flagged
regardless. So I think if we go for explicitly supporting fields, we should
probably exclude all non-aggregate classes from the callback, and leave an
irritated comment explaining why we eventually agreed that it's a good idea 😅
https://github.com/llvm/llvm-project/pull/91991
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
