[PATCH] D121176: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl.
Volodymyr Sapsai via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 8 16:31:56 PST 2022
vsapsai added a comment.
Thanks for the reviews! I'm going to address the comments, `check-lldb`, check tests failing on Debian.
================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1971
+ return false;
+ unsigned SlotsToCheck = NumArgs > 0 ? NumArgs : 1;
+ for (unsigned I = 0; I < SlotsToCheck; ++I) {
----------------
shafik wrote:
> I am curious what case are we catching here? Does `NumArgs` ever have the value `1`?
Here I'm trying to handle the case with argument-less methods. For example, `-(void)test;` has 0 arguments but we still want to compare the first slot. At the moment I don't know how to clarify the code to make the intention clearer. Can add a comment explaining the intention (though there is a test that should catch breaking code accidentally).
And to answer your question, `NumArgs` can have value `1`.
================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1989
+ ++ParamT1, ++ParamT2) {
+ if (!IsStructurallyEquivalent(Context, *ParamT1, *ParamT2))
+ return false;
----------------
a_sidorin wrote:
> Should we check if the parameter count is different?
Mismatch in the number of parameters should be caught by `NumArgs != Selector2.getNumArgs()` earlier. But I think it makes sense to add an assertion verifying `param_size` is equal at this point.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121176/new/
https://reviews.llvm.org/D121176
More information about the cfe-commits
mailing list