[PATCH] D48628: [AST] Structural equivalence of methods

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: lib/AST/ASTStructuralEquivalence.cpp:873
+
+  if (auto *Constructor1 = dyn_cast<CXXConstructorDecl>(Method1)) {
+    if (auto *Constructor2 = dyn_cast<CXXConstructorDecl>(Method2)) {
----------------
a.sidorin wrote:
> ```if (Method1->getStmtKind() != Method2->getStmtKind())
>   return false;```
> So we need to check only one declaration here and below.
What do you mean by getStmtKind? Probably getDeclKind? Anyway after the conversions it is good to check the pointers anyway, so there would be no big simplification.


================
Comment at: lib/AST/ASTStructuralEquivalence.cpp:892
+  
+  if (Method1->isOverloadedOperator() && Method2->isOverloadedOperator()) {
+    if (Method1->getOverloadedOperator() != Method2->getOverloadedOperator())
----------------
a.sidorin wrote:
> These two lines look a bit strange to me. For example, should we return false if one of the methods is an overloaded operator and other one is not? I guess these conditions need to be swapped or written like this:
> 
> ```
> if (Method1->getOverloadedOperator() != Method2->getOverloadedOperator())
>   return false;
> 
> const IdentifierInfo *Literal1 = Method1->getLiteralIdentifier();
> const IdentifierInfo *Literal2 = Method2->getLiteralIdentifier();
> if (!::IsStructurallyEquivalent(Literal1, Literal2))
>   return false;
> ```
> omitting the first condition.
The single `if` with `getOverloadedOperator` should be enough. `getLiteralIdentifier` is not applicable for methods as far as I know (even if yes it is not related to the overloaded operator) (probably it is missing from check at `FunctionDecl`).


Repository:
  rC Clang

https://reviews.llvm.org/D48628