[pooma-dev] small bugs

[Jeffrey D. Oldham]

Richard Guenther wrote:

>On Mon, 24 May 2004, ron hylton wrote:
>
>  
>
>>Here are some small bugs that perhaps should be fixed in CVS.
>>
>>in Array.h
>>
>>Array::physicalDomain() should return an object rather than a reference
>>because view engines return a temporary layout and the current behavior is
>>sometimes returning a reference to the interior of a temporary. Fix:
>>
>>  inline const Domain_t physicalDomain() const
>>    
>>
>
>Indeed.
>
>  
>
>>in IndexFunctionEngine.h
>>
>>Engine::setDomain() should set firsts_m to be consistent with the
>>constructors. Fix:
>>
>>  void setDomain(const Domain_t &dom)
>>  {
>>	domain_m = dom;
>>    for (int d = 0; d < Dim; ++d)
>>      firsts_m[d] = domain_m[d].first();
>>  }
>>    
>>
>
>Hm, I think we should rather drop the firsts_m member and change
>first(int i) to return domain_m[i].first() instead.  firsts_m isn't used
>otherwise.
>
>  
>
>>I think IndexFunctionEngine also should have an Engine::layout() member to
>>be consistent with other Engines.  The simplest possibility is:
>>
>>  inline Layout_t layout() const { return Layout_t(domain_m); }
>>    
>>
>
>Yes.
>
>  
>
>>in ForwardingEngine.h
>>
>>in struct NewEngine
>>  typedef Engine<Dim, T, CompFwd<NewEngine_t, Components> > Type_t;
>>should be
>>  typedef Engine<NewEngine_t::dimensions, T, CompFwd<NewEngine_t,
>>Components> > Type_t;
>>    
>>
>
>Yes.
>
>Compiled ok, not tested (but all those look obvious).  Ok?
>  
>
It's great to have these improvements, but let's compile the test cases 
before committing.  Doing so does not take too long and will help 
isolate the source of any such problems.  After that, please commit 
these changes.

>Richard.
>
>
>2004May25  Richard Guenther <richard.guenther at uni-tuebingen.de>
>
>	From Ron Hylton <ron_hylton at hotmail.com>
>
>	* src/Array/Array.h: don't possibly return reference to
>	temporary in physicalDomain().
>	src/Engine/IndexFunctionEngine.h: remove firsts_m member,
>	add layout() accessor.
>	src/Engine/ForwardingEngine.h: use NewEngine_t::dimensions
>	for Type_t in NewEngine traits.
>
>
>Index: Array/Array.h
>===================================================================
>RCS file: /home/pooma/Repository/r2/src/Array/Array.h,v
>retrieving revision 1.150
>diff -u -u -r1.150 Array.h
>--- Array/Array.h	2 Mar 2004 18:18:45 -0000	1.150
>+++ Array/Array.h	25 May 2004 09:35:50 -0000
>@@ -1796,7 +1796,7 @@
>
>   /// Returns the physical domain, i.e. the domain without external guards.
>
>-  inline const Domain_t& physicalDomain() const
>+  inline Domain_t physicalDomain() const
>     {
>       return engine_m.layout().innerDomain();
>     }
>Index: Engine/IndexFunctionEngine.h
>===================================================================
>RCS file: /home/pooma/Repository/r2/src/Engine/IndexFunctionEngine.h,v
>retrieving revision 1.26
>diff -u -u -r1.26 IndexFunctionEngine.h
>--- Engine/IndexFunctionEngine.h	22 Oct 2003 19:38:07 -0000	1.26
>+++ Engine/IndexFunctionEngine.h	25 May 2004 09:35:51 -0000
>@@ -124,16 +124,12 @@
>   explicit Engine(const Domain_t &domain, const Functor &f = Functor())
>   : funct_m(f), domain_m(domain)
>   {
>-    for (int d = 0; d < Dim; ++d)
>-      firsts_m[d] = domain[d].first();
>   }
>
>   template<class Layout>
>   explicit Engine(const Layout &layout, const Functor &f = Functor())
>   : funct_m(f), domain_m(layout.domain())
>   {
>-    for (int d = 0; d < Dim; ++d)
>-      firsts_m[d] = domain_m[d].first();
>   }
>
>   //---------------------------------------------------------------------------
>@@ -142,8 +138,6 @@
>   Engine(const This_t &model)
>   : funct_m(model.functor()), domain_m(model.domain())
>   {
>-    for (int d = 0; d < Dim; ++d)
>-      firsts_m[d] = model.firsts_m[d];
>   }
>
>   //---------------------------------------------------------------------------
>@@ -153,8 +147,6 @@
>   {
>     domain_m = rhs.domain();
>     funct_m = rhs.functor();
>-    for (int d = 0; d < Dim; ++d)
>-      firsts_m[d] = rhs.firsts_m[d];
>
>     return *this;
>   }
>@@ -240,7 +232,15 @@
>   inline int first(int i) const
>   {
>     PAssert(i >= 0 && i < Dim);
>-    return firsts_m[i];
>+    return domain_m[i].first();
>+  }
>+
>+  //---------------------------------------------------------------------------
>+  /// Returns the layout, which is constructed as a DomainLayout.
>+
>+  Layout_t layout() const
>+  {
>+    return Layout_t(domain_m);
>   }
>
>   //---------------------------------------------------------------------------
>@@ -253,7 +253,6 @@
>
>   Functor funct_m;
>   Domain_t domain_m;
>-  int firsts_m[Dim];
> };
>
>
>Index: Engine/ForwardingEngine.h
>===================================================================
>RCS file: /home/pooma/Repository/r2/src/Engine/ForwardingEngine.h,v
>retrieving revision 1.48
>diff -u -u -r1.48 ForwardingEngine.h
>--- Engine/ForwardingEngine.h	22 Oct 2003 19:38:07 -0000	1.48
>+++ Engine/ForwardingEngine.h	25 May 2004 09:35:51 -0000
>@@ -317,7 +317,7 @@
> struct NewEngine<Engine<Dim, T, CompFwd<Eng, Components> >, Domain>
> {
>   typedef typename NewEngine<Eng, Domain>::Type_t NewEngine_t;
>-  typedef Engine<Dim, T, CompFwd<NewEngine_t, Components> > Type_t;
>+  typedef Engine<NewEngine_t::dimensions, T, CompFwd<NewEngine_t, Components> > Type_t;
> };
>
> /**
>  
>


-- 
Jeffrey D. Oldham
oldham at codesourcery.com