erichkeane added inline comments.
================
Comment at: clang/lib/Sema/SemaType.cpp:2583
+ if (ArraySize->containsErrors()) {
+ RecoveryExpr *RE = RecoveryExpr::Create(
+ Context, ArraySize->getType(), ArraySize->getBeginLoc(),
----------------
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?
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
