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

[Aaron Ballman via Phabricator via cfe-commits]

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 vla_unevaluated(int x) {
> > > >   // Evaluates the ++x
> > > >   sizeof(int[++x]);
> > > >   // Does not evaluate the ++x
> > > >   _Alignof(int[++x]);
> > > >   _Generic((int(*)[++x])0, default : 0);
> > > >   
> > > >   return x;
> > > > }
> > > > ```
> > > > 
> > > > Also, it's a bit strange to find these VLA tests in a .cpp file. VLAs are a C++ extension and we should have some testing for them for constructs that are C++-specific, but I would have expected many of these tests to be in a C file instead.
> > > Could be there platforms (on buildbot?) where the VLA code (in cpp file) does not compile? Is it worth to have a test with VLA type alias then?
> > I have this output for the test above (without the new code), is it correct?
> > (The `++` seems to be evaluated for the `alignof` statement too.)
> > ```
> > int vla_unevaluated(int x)
> >  [B2 (ENTRY)]
> >    Succs (1): B1
> > 
> >  [B1]
> >    1: x
> >    2: ++[B1.1]
> >    3: [B1.2] (ImplicitCastExpr, LValueToRValue, int)
> >    4: sizeof(int [++x])
> >    5: x
> >    6: ++[B1.5]
> >    7: [B1.6] (ImplicitCastExpr, LValueToRValue, int)
> >    8: alignof(int [++x])
> >    9: 0
> >   10: x
> >   11: [B1.10] (ImplicitCastExpr, LValueToRValue, int)
> >   12: return [B1.11];
> >    Preds (1): B2
> >    Succs (1): B0
> > 
> >  [B0 (EXIT)]
> >    Preds (1): B1
> > 
> > ```
> > Could be there platforms (on buildbot?) where the VLA code (in cpp file) does not compile? Is it worth to have a test with VLA type alias then?
> 
> That's unlikely; the problems with VLAs in C++ are IIRC about the bureaucracy of the Standard (i.e., they couldn't solve enough cornercases, like whether `T<int[x]>` and `T<int[y]>` the same template when `x == y` at run-time?), not about platform support.
> Could be there platforms (on buildbot?) where the VLA code (in cpp file) does not compile? Is it worth to have a test with VLA type alias then?

I think the type alias test is a good idea (as are any other C++-specific tests) because we support VLAs in C++ mode as an extension, my concern was more that this is a C feature and so if I was hunting for tests for VLAs, I'd look  in a .c file first. It's not a huge concern, more like a style nit.

> (The ++ seems to be evaluated for the alignof statement too.)

That seems wrong per C17 6.5.3.4p3. It also doesn't seem to match the behavior I'm seeing: https://godbolt.org/z/rVaGnb



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77809/new/

https://reviews.llvm.org/D77809