[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG007dd12d5468: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl (authored by martong). Repository: rG LLVM Github

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong added a comment. > Currently, we are totally inconsistent about the diagnostics. Either we > should emit a diagnostic before all return false or we should not ever emit > any diags. The diagnostics in their current form are misleading, because > there could be many notes missing. I am

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

teemperor added a comment. > I am not sure how much do you guys value these diags in LLDB I think we don't even display them most of the time (IIUC they go to the diagnostic engine which is not always hooked up for the ASTs in LLDB as only a few actually come from Clang). Repository: rG

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88665/new/ https://reviews.llvm.org/D88665

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

shafik added a comment. In D88665#2308366 , @martong wrote: >> In D88665#2307562 , @shafik wrote: >> >>> So was the bug we were saying there were falsely equivalent or falsely not >>> equivalent? >> >> I think the

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong updated this revision to Diff 295800. martong added a comment. - Delegate to BitWidth Expr matching in case of BitFields Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88665/new/ https://reviews.llvm.org/D88665 Files:

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong added a comment. > In 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 :) Yes. More specifically, the call of `getBitWidthValue()`

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

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,

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

shafik added a comment. So was the bug we were saying there were falsely equivalent or falsely not equivalent? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88665/new/ https://reviews.llvm.org/D88665

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong added a comment. The added test causes the below assertion in the baseline (w/o the fix): ASTTests: ../../git/llvm-project/clang/lib/AST/ExprConstant.cpp:14543: llvm::APSInt clang::Expr::EvaluateKnownConstInt(const clang::ASTContext&, llvm::SmallVectorImpl >*) const: Assertion

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

martong created this revision. martong added reviewers: balazske, teemperor, shafik, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. martong requested review of this revision. Repository: