[PATCH] D62329: [ASTImporter] Structural eq: handle DependentScopeDeclRefExpr

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 28 07:32:06 PDT 2019


martong marked 8 inline comments as done.
martong added inline comments.


================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:124
+  case DeclarationName::CXXConversionFunctionName:
+    return true;
+
----------------
a_sidorin wrote:
> Should we check the equivalence of getCXXNameType() in such cases?
Good catch, thanks!


================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:127
+  case DeclarationName::CXXDeductionGuideName:
+    return IsStructurallyEquivalent(
+        Name1.getCXXDeductionGuideTemplate()->getDeclName(),
----------------
a_sidorin wrote:
> Should we check the equivalence of the whole Name1.getCXXDeductionGuideTemplate() (with the template arguments)?
Good catch, thanks!


================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:147
+
+  return true;
+}
----------------
a_sidorin wrote:
> llvm_unreachable()?
Good catch, thanks!


================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:163
+    return IsStructurallyEquivalent(Context, DE1->getQualifier(),
+                                    DE2->getQualifier());
+  }
----------------
a_sidorin wrote:
> Should we compare TemplateArgs (getTemplateArgs) somehow?
No, that is not needed. Because `getQualifier()` returns with a `NestedNameSpecifier` and then in the appropriare overload of IsStructurallyEquivalent we will investigate further the type together with the template params:
```
 213   case NestedNameSpecifier::TypeSpec:
 214   case NestedNameSpecifier::TypeSpecWithTemplate:
 215     return IsStructurallyEquivalent(Context, QualType(NNS1->getAsType(), 0),
 216                                     QualType(NNS2->getAsType(), 0));

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62329





More information about the cfe-commits mailing list