[PATCH] D39886: [ASTImporter] Fix wrong conflict detections for unnamed structures

Takafumi Kubota via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 21 05:30:12 PST 2017


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.
Then, I think https://reviews.llvm.org/D30876's solution is more conservative.
In fact,  simply omitting the conflict check for all unnamed structures works well in my experience.
I have not hit the case where two unnamed structs have the same `Index`.
However, according to two past patches (1cef459 <https://github.com/llvm-mirror/clang/commit/1cef45955d87dde48c78b6878dc0ee67a3404b6c#diff-7f5f26dbdb3322c6308b1018ff0dbe5a> and 11ce5fe <https://github.com/llvm-mirror/clang/commit/11ce5fe7d8dda29f3d093e5759a7476b140e3241#diff-7f5f26dbdb3322c6308b1018ff0dbe5a> ), he clearly added the anonymous structs/unions to the checking path.
So, I guess there is an example where ASTImporter has to handle them rather than simply skips them.
https://reviews.llvm.org/D30876 more reflects the policy of these patches.

Furthermore, I also agree with the fix of findUntaggedStructOrUnionIndex(), although findUntaggedStructOrUnionIndex() is not in ASTImporter.cpp anymore.
The function exists in ASTStructuralEquivalence.cpp insead.

Sorry for this redundant patch.


https://reviews.llvm.org/D39886





More information about the cfe-commits mailing list