[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

Akira Hatanaka via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 5 08:14:40 PDT 2023


ahatanak added a comment.

In D151523#4374808 <https://reviews.llvm.org/D151523#4374808>, @vsapsai wrote:

> Kinda a follow-up to D121176 <https://reviews.llvm.org/D121176>.
>
> One big alternative that I've considered is to store interface name in `ObjCCategoryDecl` because when we parse `@interface InterfaceName(CategoryName)` we know the interface name. The intention was to allow
>
>   @interface A(X) @end
>
> and
>
>   @interface A @end
>   @interface A(X) @end
>
> as equivalent. But I'm not convinced it is the right call and that's why it doesn't justify the extra effort.

Are there any cases you are aware of where this change would fix a bug? In any case, it sounds like that is something that can be done as a follow-up patch after fixing the crash.



================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:2062
+                          *Intf2 = D2->getClassInterface();
+  if ((Intf1 != nullptr) != (Intf2 != nullptr))
+    return false;
----------------
vsapsai wrote:
> shafik wrote:
> > I think this would be easier to read if you checked `Intf1 != Intf2` and then checked for `nullptr`
> I am totally up to the style that is more readable and consistent. I was just trying to mimic the check for `Template1` and `Template2`. I agree that 1 (**one**) datapoint isn't representative, so I can check this file more thoroughly for the prevalent style. Do you have any other places in mind that are worth checking? I'll look for something more representative but it would help if you have something in mind already.
`!Intf1 != !Intf2` should work too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151523/new/

https://reviews.llvm.org/D151523



More information about the cfe-commits mailing list