[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-25 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. @balazske , I tested these changes - again - and it all seems to work for me. I would have preferred we do not see the build status failures before doing this, but maybe we can be sure to avoid this next time (because I'll want to test again before our final merge).

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-23 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. In D145868#4189574 , @vabridgers wrote: > Hi @balazske , all LIT and unittests pass with this change. By your logic, > then we are missing some LIT or unittest cases that support your statement. > Can you think of a case that

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-20 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Hi @balazske . Can we go back to the original proposed fix and treat the new issue separately? We have an internal crash open that is corrected by the original patch I proposed, and passes all LITs and unit tests. I think it would be better to separate these

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-15 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 505481. balazske added a comment. Added a simple test and another test. Instead of calling getTypeDeclType only the reuse of existing type is done (this was the part that fixes the problem). In this way the underlying type is still imported. The result is

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-14 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. The problem is somewhat bigger and not fast to fix. This test shows what is problematic: TEST_P(ASTImporterOptionSpecificTestBase, ImportExistingTypedefToUnnamedRecordPtr) { const char *Code = R"( typedef const struct { int fff; } * const

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-13 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Hi @balazske , all LIT and unittests pass with this change. By your logic, then we are missing some LIT or unittest cases that support your statement. Can you think of a case that demonstrates this? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-13 Thread Balázs Kéri via Phabricator via cfe-commits
balazske commandeered this revision. balazske edited reviewers, added: vabridgers; removed: balazske. balazske added a comment. Herald added subscribers: steakhal, Szelethus, dkrupp. My opinion is that we can not omit importing the "underlying type". The `TypedefType` has the type of the

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-13 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. @balazske and I discussed, he will be commandeering this patch to improve. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145868/new/ https://reviews.llvm.org/D145868 ___

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

2023-03-12 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added reviewers: a.sidorin, shafik, donat.nagy, gamesh411, balazske. Herald added a subscriber: martong. Herald added a project: All. vabridgers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.