[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

This revision was automatically updated to reflect the committed changes. Closed by commit rC352050: [ASTImporter] Fix inequality of functions with different attributes (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D53699?vs=178060=183307#toc

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Thank you for adding the additional test. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong added a comment. Ping @shafik, I have addressed you comments, could you please take another look? If you don't have any objections, could you please approve? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong marked 4 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTStructuralEquivalence.cpp:302 +/// is inspired by ASTContext::mergeFunctionTypes(), we compare calling +/// conventions bits but must not compare some other bits, e.g. the noreturn

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong updated this revision to Diff 178060. martong marked 2 inline comments as done. martong added a comment. - Add more tests and simplify comment Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699 Files:

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

shafik added inline comments. Comment at: lib/AST/ASTStructuralEquivalence.cpp:302 +/// is inspired by ASTContext::mergeFunctionTypes(), we compare calling +/// conventions bits but must not compare some other bits, e.g. the noreturn +/// bit. This comment is

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong added a comment. Hi @a_sidorin , I have updated the patch as you suggested, to check the equivalence based on `ASTContext::mergeFunctionTypes()`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong updated this revision to Diff 175647. martong added a comment. - Use ExtInfo in structural equivalency Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699 Files: lib/AST/ASTStructuralEquivalence.cpp

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong added a comment. Herald added a subscriber: gamesh411. That's a good point. I agree, we should check some bits (calling convention bits) but not all (e.g. noreturn bit). I am going to create another patch which replaces this. Repository: rC Clang https://reviews.llvm.org/D53699

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

a_sidorin added a comment. Hi Gabor, Thank you for the patch. The reason for this change looks clear. However, I don't think omitting this comparison completely is what we want here. Instead, we can do a dance similar to `ASTContext::mergeFunctionTypes()` where all attributes but NoReturn are

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. FunctionType::ExtInfo holds such properties of a function which are needed mostly for code gen. We should not compare these