Page MenuHomePhabricator

[clang][AST] Add support for BindingDecl to ASTImporter.
Needs ReviewPublic

Authored by balazske on May 14 2021, 6:02 AM.

Details

Diff Detail

Event Timeline

balazske created this revision.May 14 2021, 6:02 AM
balazske requested review of this revision.May 14 2021, 6:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 14 2021, 6:02 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
shafik added inline comments.May 17 2021, 10:26 AM
clang/unittests/AST/ASTImporterTest.cpp
873

Can we add a larger variety of tests? Given how simple the BindingDecl case seems to be I don't expect any surprises but it would be good to verify that cases like mixed types, references and bit-fields work as expected.

balazske updated this revision to Diff 346169.May 18 2021, 7:06 AM

Improved the test.

This change in itself is not enough to support BindingDecl. The tests pass but the resulting AST is not correct because DecompositionDecl is not imported yet. Still no visible problem happens, the DecompositionDecl is just visited and imported as a VarDecl. The import of DecompositionDecl can be added later (but I am not sure if without it the new import of BindingDecl will not cause hidden problems).

clang/unittests/AST/ASTImporterTest.cpp
873

Improved the test variety a bit. (std::tuple is more difficult to handle because need for namespace std.)

I like it, though I've found a nit.

clang/lib/AST/ASTImporter.cpp
2301

Should we use rather addDeclToContexts ?

balazske updated this revision to Diff 352372.Wed, Jun 16, 1:30 AM

Using addDeclToContexts.