tk1012 added a comment.
Hello Aleksei,
Unfortunately, I find the related problem with the unnamed structs/unions, even
if I apply https://reviews.llvm.org/D39886.
For example, in PostgreSQL, there is a part of code like below.
typedef struct { int a; } b;
struct { const char *x; } y;
a.sidorin added a comment.
No need to apologize. Thank you for your work anyway. Even if the bug-fixing
part of this patch will be omitted, I'd still like to add your tests into the
test suite.
https://reviews.llvm.org/D39886
___
cfe-commits maili
tk1012 added a comment.
Oh, yes.
https://reviews.llvm.org/D30876 is motivated by the same problem.
The notable difference between this patch and https://reviews.llvm.org/D30876
is that ASTImporter should check the conflict resolution for the unnamed
structs/unions in a record context or not.
T
a.sidorin requested changes to this revision.
a.sidorin added a comment.
This revision now requires changes to proceed.
Hello Takafumi,
Could you investigate the patch https://reviews.llvm.org/D30876? It should fix
same issue. However, it also handles conflict resolution. I accidentally forgot
a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.
Hello Takafumi,
I think you are correct. So, these are not unnamed structures that need
conflict resolution but declarations that instantiate the type. Therefore, we
can omit both looku
tk1012 added inline comments.
Comment at: lib/AST/ASTImporter.cpp:1665
// they occur in the same location in the context records.
if (Optional Index1 =
StructuralEquivalenceContext::findUntaggedStructOrUnionIndex(
Anonymou
tk1012 marked an inline comment as done.
tk1012 added a comment.
Fix the broken indentation.
https://reviews.llvm.org/D39886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tk1012 updated this revision to Diff 123493.
tk1012 added a comment.
Herald added a subscriber: rnkovacs.
Hello,
I update the diff to solve the below thing.
> 1. Are import conflicts for anonymous structures resolved correctly?
Before I describe the updates, I want to detail the difference betw
tk1012 added a comment.
Hallo Aleksei and Gábor,
Thank you for your response.
> 1. Are import conflicts for anonymous structures resolved correctly?
In fact, this patch only fixes the unnamed structures that are not anonymous.
In https://reviews.llvm.org/D39886#923188, @tk1012 wrote:
>
I ad
tk1012 added a comment.
Comment at: lib/AST/ASTImporter.cpp:1634
RecordDecl *PrevDecl = nullptr;
- if (!DC->isFunctionOrMethod()) {
+ if (!DC->isFunctionOrMethod() && SearchName.getAsString() != "") {
SmallVector ConflictingDecls;
According to includ
a.sidorin added a comment.
Hello Takafumi,
Thank you for this patch. Looks like you're trying to disable lookup for
similar structures if the structure is anonymous but there are two things I'm
worrying about this solution.
1. Are import conflicts for anonymous structures resolved correctly?
2
xazax.hun added a reviewer: doug.gregor.
xazax.hun added a comment.
Doug added anonymous structure handling, added as a reviewer in case he wants
to have a look.
https://reviews.llvm.org/D39886
___
cfe-commits mailing list
cfe-commits@lists.llvm.or
tk1012 created this revision.
This patch fixes wrong conflict detections for unnamed structures.
Current ASTImporter mistakenly identifies two different unnamed structs as the
same one.
This is because ASTImporter checks the name of each RecordDecl for the conflict
identification and the both of
13 matches
Mail list logo
