Page MenuHomePhabricator

[ASTImporter] Extend lookup logic in class templates
ClosedPublic

Authored by martong on May 2 2018, 3:49 AM.

Details

Summary

During import of a class template, lookup may find a forward declaration and structural match falsely reports equivalency in between a fwd decl and a definition. This results that some definitions are not imported if we had imported a fwd decl previously. This patch gives a fix.

Diff Detail

Repository
rC Clang

Event Timeline

martong created this revision.May 2 2018, 3:49 AM

During import of a class template, lookup may find a forward declaration and structural match falsely reports equivalency in between a fwd decl and a definition.

This can happen when the class to be imported does not have any data members. Structural equivalency check the data members only (and not any of the member functions).

Hello Gabor,

Thank you for the patch. It looks mostly good, but I have a question: should we add this new declaration to the redeclaration chain like we do it for RecordDecls?

should we add this new declaration to the redeclaration chain like we do it for RecordDecls?

I think, on a long term we should. Otherwise we could loose e.g. C++11 attributes which are attached to the forward declaration only.
However, I'd do that as a separate commit, because that would require some independent changes and tests, also other decl kinds like ClassTemplateSepcializationDecl may be affected as well by that.

a.sidorin accepted this revision.May 14 2018, 8:31 AM

should we add this new declaration to the redeclaration chain like we do it for RecordDecls?

I think, on a long term we should. Otherwise we could loose e.g. C++11 attributes which are attached to the forward declaration only.
However, I'd do that as a separate commit, because that would require some independent changes and tests, also other decl kinds like ClassTemplateSepcializationDecl may be affected as well by that.

I'm fine with this. But a FIXME would be very appreciated.

This revision is now accepted and ready to land.May 14 2018, 8:31 AM
martong updated this revision to Diff 146765.May 15 2018, 2:36 AM
  • Add FIXME

Hi Aleksei,

Added the FIXME, can you help me with committing this?

This revision was automatically updated to reflect the committed changes.