HerrCai0907 added inline comments.
================ Comment at: clang/lib/Sema/SemaType.cpp:2583 + if (ArraySize->containsErrors()) { + RecoveryExpr *RE = RecoveryExpr::Create( + Context, ArraySize->getType(), ArraySize->getBeginLoc(), ---------------- erichkeane wrote: > HerrCai0907 wrote: > > erichkeane wrote: > > > Actually thinking further... rather than create a NEW RecoveryExpr, is > > > there a problem KEEPING the ArraySize expression? It'd likely give more > > > information/keep more AST consistency. > > > > > > ALSO, creating this as size-0 has some implications we probably don't > > > want. I wonder if a different for the placeholder would be better? > > > > > > I'D ALSO suggest hoisting this ArraySize->containsError out of the > > > else-if and into its own branch (rather than inside the dependent checks). > > > KEEPING the ArraySize expression > > Agree. > > > > > creating this as size-0 has some implications we probably don't want. > > Do you think we can modify it as 1? But I don't find a better way to > > identifier it as undef or other. > > > > > hoisting this ArraySize->containsError out of the else-if > > Done > I don't have good evidence for what else we could do (whether an incomplete > or variable type would be better), but at least 1 is 'legal', so perhaps we > can live with this for now and see if there is any fallout. > > What happens if we try to 'merge' a valid and an invalid here? Does it make > the experience worse? So same example you added below, but the 1st one has > no error in its brackets? I add test case for this scenario. I think it will not confuse developer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149612/new/ https://reviews.llvm.org/D149612 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits