xazax.hun added inline comments.
================ Comment at: clang/lib/Analysis/CFG.cpp:2855 + VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl()); ---------------- balazske wrote: > Szelethus wrote: > > How about `using`? How about some other shenanigans that obscure the size > > of the VLA? Can't we just retrieve the size from > > `VariableArrayType::getSizeExpr` after seeing this `VarDecl`, and add that > > to the block? > The current code does that, it gets the size with `getSizeExpr` and adds it. > The loop is needed to support multiple array dimensions. > > To support every other type that can contain VLA a similar code is needed as > in `CodeGenFunction::EmitVariablyModifiedType` (go over the type chain and > look not only for `VariableArrayType`). The current code works only if there > is typedef (or type alias) with a plain VLA, maybe embedded into another. > If support for every other case is needed, it is a change on its own because > the code that generates CFG for VLA declaration is wrong too (later in the > same function, the new code is copied from that). For example `void > (*vla)(int[x]);` does not work now (`x` is not included in CFG). This is > better to do in a later patch, for now this more restricted functionality is > better than nothing (it is not wrong, only do not support all, probably > seldom used, cases). I'd prefer to have some FIXME comments in this patch to document what is missing. Moreover, it would be great to have a test case for `void (*vla)(int[x]);` with a similar FIXME comment of the expected behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
