teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.
Shouldn't this compare the actual width expressions? In other words, this patch
wouldn't pass the test below:
TEST_F(StructuralEquivalenceTemplateTest, DependentFieldDeclDifferentVal) {
const char *Code1 = "template <class A, class B> class foo { int a :
sizeof(A); };";
const char *Code2 = "template <class A, class B> class foo { int a :
sizeof(B); };";
auto t = makeNamedDecls(Code1, Code2, Lang_CXX03);
EXPECT_FALSE(testStructuralMatch(t));
}
I don't want to derail the simple crash fix here, but I'm tempted to say that
we might as well just simplify all this to `return
IsStructurallyEquivalent(Context, Field1->getBitWidth(),
Field2->getBitWidth());`. The nice diagnostic could live behind behind a check
that ensures that both widths are not value-dependent.
If you want to keep this simple because you want to backport it I would also be
fine with the current patch (not crashing is clearly an improvement)
In D88665#2307562 <https://reviews.llvm.org/D88665#2307562>, @shafik wrote:
> So was the bug we were saying there were falsely equivalent or falsely not
> equivalent?
I think the bug is the crash :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88665/new/
https://reviews.llvm.org/D88665
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
