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

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

balazske marked an inline comment as done.
balazske added inline comments.


================
Comment at: unittests/AST/StructuralEquivalenceTest.cpp:489
+
+TEST_F(StructuralEquivalenceRecordTest, DISABLED_Methods) {
+  auto t = makeNamedDecls(
----------------
a_sidorin wrote:
> balazske wrote:
> > a_sidorin wrote:
> > > Could you add a comment why this test is disabled?
> > Methods are not checked, there was no decision about to include this check or not. The problem was related to performance overhead and if order-independent check of methods is needed. (ASTImporter should keep order of imported fields and methods.) (This test is about equivalence of `foo`.)
> You mean that imported decl have other order of methods? Do you mean implicit methods (because I see only a single method here)? If so, could you please note this in the comment?
The problem with the ordering is in general at structural equivalence check. If a complex structure is imported with ASTImporter the ordering of fields and methods may change in the imported class (relative to the to-be imported) after the import. Either order-independent check is needed in structural equivalence check to match two classes with same methods but in different order, or import should not change ordering. Currently none of these is implemented, methods are not checked at all. The `foo`s in this test should be non-equivalent because one different method `x`.


Repository:
  rC Clang

https://reviews.llvm.org/D48628