void marked 4 inline comments as done.
void added a comment.
In D148381#4625462 <https://reviews.llvm.org/D148381#4625462>, @nickdesaulniers
wrote:
> I assume you plan to add some clang CodeGen tests at some point?
Yes. :-)
================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:531-532
- /// True if CodeGen currently emits code implementing sanitizer checks.
- bool IsSanitizerScope = false;
+ /// Non-zero if CodeGen currently emits code implementing sanitizer checks.
+ unsigned IsSanitizerScope = 0;
----------------
nickdesaulniers wrote:
> I don't understand this change. Grepping for `IsSanitizerScope` doesn't show
> any changed references to how this variable is used. Intentional?
Ah! Thanks for catching this. I had trouble with this scoping mechanism and for
a while used a reference counting method to get around it, but put it back to
its previous impl, except for this. I still think the `SanitizerScope` needs
work, but that's beyond this project.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:17911
+ return FD;
+ }
+
----------------
nickdesaulniers wrote:
> unnecessary curly braces?
I prefer it this way in this case (actually, I wish C++ had a way to combine
those two `if`'s into one). But it's not a big deal.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:17931
+ auto It = llvm::find_if(RD->fields(), [&](const FieldDecl *Field) {
+ return Field->getName() == ECA->getCountedByField()->getName();
+ });
----------------
rapidsna wrote:
> What happens in this corner case where the `Field` is actually the field that
> the attribute appertains to?
> ```
> struct foo {
> int count;
> int counted[__attribute__((counted_by(counted)))];
> };
>
> ```
That should be an error. I'll add that as a check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148381/new/
https://reviews.llvm.org/D148381
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
