[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-28 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc5d83cdca457: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr. (authored by balazske). Changed prior to commit: https://reviews.llvm.org/D120824?vs=414642=418521#toc Repository: rG

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-22 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. Ping I had a question about if the lit-test is needed with the new unit tests (that contain essentially similar code). But if there is no answer I will commit it with all tests, more test is always better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-11 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. The last unit test is not much different from the lit test, I could not find more simple code to get the wanted result. Is it still good to have the lit test too? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120824/new/

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-11 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 414642. balazske added a comment. Added unit tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120824/new/ https://reviews.llvm.org/D120824 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-09 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D120824#3369563 , @balazske wrote: > > I think that the problem may be related to the fact that the in-class > initializer is not used by the code in the "To" AST (in > //ctu-cxxdefaultinitexpr.cpp// the problem is with

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-09 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. I think that the problem may be related to the fact that the in-class initializer is not used by the code in the "To" AST (in //ctu-cxxdefaultinitexpr.cpp// the problem is with `QMultiHash::d` field). Probably the expression node in the field is set only if it is used

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-09 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Okay, thanks for the explanation. > The in-class initializer expression is not always stored in the AST, in the > ToTU it is missing initially. The field has the flag set that it contains > in-class initializer but the actual expression is not set yet (probably >

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-09 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. In D120824#3369407 , @martong wrote: > So, my understanding is that the issue stems from the fact that > `hasInClassInitializer()` gave inconsistent results with > `getInClassInitializer()` for previously imported nodes. Not

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-09 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. So, my understanding is that the issue stems from the fact that `hasInClassInitializer()` gave inconsistent results with `getInClassInitializer()` for previously imported nodes. Please

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-03 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. Looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120824/new/ https://reviews.llvm.org/D120824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-03 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 412665. balazske added a comment. Improved the test to fix failure on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120824/new/ https://reviews.llvm.org/D120824 Files:

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

2022-03-02 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision. Herald added subscribers: steakhal, martong, gamesh411, Szelethus, dkrupp. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a reviewer: Szelethus. Herald added a project: All. balazske requested review of this revision. Herald added