[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

This revision was automatically updated to reflect the committed changes. Closed by commit rG3b9b3d56efaa: [Analyzer] Include typedef statements in CFG build. (authored by balazske). Changed prior to commit: https://reviews.llvm.org/D77809?vs=259820=260269#toc Repository: rG LLVM Github

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2855 + VA = FindVA(VA->getElementType().getTypePtr())) { + if (CFGBlock *newBlock = addStmt(VA->getSizeExpr())) +LastBlock = newBlock; balazske wrote: >

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2855 + VA = FindVA(VA->getElementType().getTypePtr())) { + if (CFGBlock *newBlock = addStmt(VA->getSizeExpr())) +LastBlock = newBlock; aaron.ballman wrote: > newBlock ->

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 259820. balazske marked an inline comment as done. balazske added a comment. - Separating test files, fixing alignof problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

xazax.hun accepted this revision. xazax.hun added a comment. Overall looks good for me, thanks for tackling this problem! I think this should be good to go once Eli's comment is fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

efriedma added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2928 + // FIXME: This does not find the VLA if it is embedded in other types, + // like here: `void (*vla)(int[x]);` for (const VariableArrayType* VA = FindVA(VD->getType().getTypePtr());

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with a minor nit, thank you! Comment at: clang/lib/Analysis/CFG.cpp:2855 + VA = FindVA(VA->getElementType().getTypePtr())) { + if (CFGBlock *newBlock = addStmt(VA->getSizeExpr())) +

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 259584. balazske added a comment. - Separating test files, fixing alignof problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809 Files: clang/lib/Analysis/CFG.cpp

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske added a comment. In D77809#1999050 , @xazax.hun wrote: > Actually, sorry. I just realized that the alignof problem is introduced by > this patch. I'd love to see the solution committed together with this patch > (it could be a separate patch

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

xazax.hun added a comment. Actually, sorry. I just realized that the alignof problem is introduced by this patch. I'd love to see the solution committed together with this patch (it could be a separate patch but preferably, they should be committed together.) Repository: rG LLVM Github

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) balazske wrote: > aaron.ballman wrote: > > NoQ wrote: > > > balazske wrote: > > > > balazske wrote: > > > > > aaron.ballman wrote: > > >

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) aaron.ballman wrote: > NoQ wrote: > > balazske wrote: > > > balazske wrote: > > > > aaron.ballman wrote: > > > > > Can you also add tests

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Thanks, this looks much better now. Could you also update the description of the revision to match the current status? (E.g. type aliases are now supported.) If you do not plan to solve

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 259513. balazske marked 5 inline comments as done. balazske added a comment. Updated tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809 Files:

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) NoQ wrote: > balazske wrote: > > balazske wrote: > > > aaron.ballman wrote: > > > > Can you also add tests for: > > > > ``` > > > > int

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

NoQ added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) balazske wrote: > balazske wrote: > > aaron.ballman wrote: > > > Can you also add tests for: > > > ``` > > > int vla_unevaluated(int x) { > > >

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) aaron.ballman wrote: > Can you also add tests for: > ``` > int vla_unevaluated(int x) { > //

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) balazske wrote: > aaron.ballman wrote: > > Can you also add tests for: > > ``` > > int

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) Can you also add tests for: ``` int vla_unevaluated(int x) { // Evaluates the ++x sizeof(int[++x]); // Does not evaluate the ++x

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 258011. balazske added a comment. - Adding tests. - Type alias is supported. - Updated comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809 Files:

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2855 + VarDecl *VD = dyn_cast(DS->getSingleDecl()); balazske wrote: > Szelethus wrote: > > How about `using`? How about some other shenanigans that obscure the size > > of the VLA?

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2855 + VarDecl *VD = dyn_cast(DS->getSingleDecl()); Szelethus wrote: > How about `using`? How about some other shenanigans that obscure the

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske added a comment. > I also see that the typedef constitutes a `DeclStmt` in the AST. Can we > include that in the CFG as well? In the static analyzer that'll be the place > where we actually start tracking that typedef, because without it the size > expression value will be quickly

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

NoQ added a comment. In D77809#1984689 , @efriedma wrote: > > Where is the size expression actually evaluated? Is it evaluated at the > > point of the typedef or at the point of the variable definition? > > At the point of the typedef. Whoa, indeed.

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

efriedma added a comment. > Where is the size expression actually evaluated? Is it evaluated at the point > of the typedef or at the point of the variable definition? At the point of the typedef. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

xazax.hun requested changes to this revision. xazax.hun added a comment. This revision now requires changes to proceed. I am not an expert when it comes to VLAs but I do see some problems here. First of all, we do not want to include typedef statements in the CFG as they are noops in terms of

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

Szelethus added reviewers: NoQ, xazax.hun. Szelethus added a subscriber: NoQ. Szelethus added a comment. Herald added a subscriber: rnkovacs. Adding @NoQ and @xazax.hun , I don't have much experience building CFGs. Comment at: clang/lib/Analysis/CFG.cpp:2855 + VarDecl *VD =

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske marked an inline comment as done. balazske added a comment. Without the new code the CFG dump for `vla_typedef` contains: [B1] 1: VLA vla; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske updated this revision to Diff 257330. balazske added a comment. - Added tests. - Updated a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https://reviews.llvm.org/D77809 Files: clang/lib/Analysis/CFG.cpp

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

Szelethus added a comment. In D77809#1979911 , @balazske wrote: > I looked at the graphical CFG dump and observed that the added code makes the > same parts appear that are there in a VLA declaration but missing if a VLA > typedef is used. Could add a

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske added a comment. I looked at the graphical CFG dump and observed that the added code makes the same parts appear that are there in a VLA declaration but missing if a VLA typedef is used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

efriedma removed a reviewer: eli.friedman. efriedma added a comment. Looks roughly fine to me, but I'm not an appropriate reviewer for static analyzer code. Comment at: clang/lib/Analysis/CFG.cpp:2859 // Of everything that can be declared in a DeclStmt, only VarDecls

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

Szelethus added a comment. In D77809#1972232 , @martong wrote: > There must be a way to dump the CFG. Perhaps you could add a test that dumps > the CFG of a given source and then by using `FileCheck` we could see the > exact feature that is missing.

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

martong added a comment. There must be a way to dump the CFG. Perhaps you could add a test that dumps the CFG of a given source and then by using `FileCheck` we could see the exact feature that is missing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

balazske created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. Array size