rjmccall added a comment.
This is great work, thank you.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5349
+def note_because_captured_by_block : Note<
+ "because it is captured by a block used in its own initializer">;
+
----------------
This will read okay on the command line, but it'll be awkward in most IDEs that
consume diagnostics. You should report the problem fairly completely in the
main diagnostic and then just say something like "captured in block here" in
the note.
================
Comment at: clang/lib/AST/Decl.cpp:2491
+bool VarDecl::isCapturedByOwnInit() const {
+ return hasAttr<BlocksAttr>() && NonParmVarDeclBits.CapturedByOwnInit;
+}
----------------
You should check `isEscapingByref()` here rather than just `hasAttr`; if none
of the capturing blocks are escaping, we don't need to worry about this. Or
are you handling this implicitly in your pass during escape-marking? That
seems subtle.
================
Comment at: clang/lib/AST/Decl.cpp:2498
+ return isCapturedByOwnInit() &&
+ isa<RecordType>(getType().getAtomicUnqualifiedType());
+}
----------------
You should use `->is<RecordType>()` instead of `isa`, since the latter doesn't
handle e.g. typedefs.
The reference to TEK_Aggregate is inappropriate here; it's okay to just talk
about "aggregates" under the idea that anything else could validly just be
zero-initialized prior to the capture.
================
Comment at: clang/lib/CodeGen/CGBlocks.cpp:2754
+
+ BuildBlockRelease(addr.getPointer(), flags, false);
+}
----------------
I think there's some tooling that will be slightly happier if you release the
value in `out` instead of `addr`. I believe we do emit a different function
call to enable that in unoptimized builds.
You should add comments describing what you're doing here. The explanation
from the commit description's pretty good.
================
Comment at: clang/lib/Sema/Sema.cpp:1949
+ return nullptr;
+}
+
----------------
There is an EvaluatedExprVisitor class which might make this easier, and which
will definitely eliminate some false positives.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90434/new/
https://reviews.llvm.org/D90434
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
