[PATCH] D35471: [Polly] [RFC] Calculate AST expression type

[Maximilian Falkenstein via Phabricator via llvm-commits]

maxf added inline comments.


================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:1029
 
+    ExprBuilder.setFixedSizeMode(true);
     Value *NumElements = ExprBuilder.create(Res);
----------------
bollu wrote:
> philip.pfaffe wrote:
> > bollu wrote:
> > > Can we create a wrapper that does this? Example,
> > > 
> > > ```lang=cpp
> > > template<typename FTy>
> > > auto scopeFixedSizeMode(FTy F, IslExprBuilder &ExprBuilder) -> decltype(FTy()) typename std::enable_if<!std::is_same<decltype(std::declval<FTy>()()), void>::value,
> > >                  decltype(std::declval<FTy>()())>::type
> > > scopedFixedSizeMode(FTy F) {
> > >     ExprBuilder.setBuildMode(true);
> > >     auto Ret = F();
> > >     ExprBuilder.setBuildMode(false);
> > >     return Ret;
> > > }
> > > 
> > > template <typename FTy>
> > > typename std::enable_if<std::is_same<decltype(std::declval<FTy>()()), void>::value, void>::type
> > > scopedFixedSizeMode(FTy F) {
> > >     ExprBuilder.setBuildMode(true);
> > >     F();
> > >     ExprBuilder.setBuildMode(false);
> > > }
> > > ```
> > > 
> > > The wrapper right now is crazy, but there has to be a way for this to be more "canonical".
> > Here's a way to express the same thing more nicely:
> > 
> > ```lang=c++
> > template<typename T> T Foo(std::function<T()> F) {                              
> >   auto R = F();                                                                 
> >   return R;                                                                     
> > }                                                                               
> >                                                                                 
> > void Foo(std::function<void()> F) {                                             
> >   F();                                                                          
> > }
> > ```
> This should let us write
> 
> 
> ```lang=c++
> ExprBuilder.setFixedSizeMode(true);
> Value *NumElements = ExprBuilder.create(Res);
> ExprBuilder.setFixedSizeMode(false);
> ```
> 
> as:
> 
> ```lang=c++
> Value *NumElements = scopedFixedSizeMode([&] () { ExprBuilder.create(Res); });
> ```
> 
> I primarily want this because it's much less error prone.
Sounds good. We should probably also think about doing this for setTrackOverflow (which originally convinced me to use a simple flag for this).


https://reviews.llvm.org/D35471