[prev in list] [next in list] [prev in thread] [next in thread] 

List:       enlightenment-devel
Subject:    Re: [E-devel] Eo: potential improvements
From:       Jérémy Zurcher <jeremy () asynk ! ch>
Date:       2013-07-29 15:16:07
Message-ID: 20130729151607.GA4639 () asynk ! ch
[Download RAW message or body]

On Monday 29 July 2013  16:03, Tom Hacohen wrote :
> On 29/07/13 16:04, Jérémy Zurcher wrote:
> > On Monday 29 July 2013  10:31, Lucas De Marchi wrote :
> > > On Sat, Jul 27, 2013 at 5:54 PM, Jérémy Zurcher <jeremy@asynk.ch> wrote:
> > > > On Saturday 27 July 2013  11:10, Carsten Haitzler wrote :
> > > > > On Fri, 26 Jul 2013 14:57:28 -0300 Lucas De Marchi
> > > > > <lucas.demarchi@profusion.mobi> said:
> > > > > 
> > > > > > On Thu, Jul 25, 2013 at 8:54 PM, Carsten Haitzler <raster@rasterman.com>
> > > > > > wrote:
> > > > > > > On Thu, 25 Jul 2013 14:58:30 -0300 Gustavo Sverzut Barbieri
> > > > > > > <barbieri@profusion.mobi> said:
> > > > > > > 
> > > > > > > > On Thu, Jul 25, 2013 at 1:46 PM, Tom Hacohen \
> > > > > > > > <tom.hacohen@samsung.com> wrote:
> > > > > > > > > On 24/07/13 03:09, Carsten Haitzler (The Rasterman) wrote:
> > > > > > > > > > On Tue, 23 Jul 2013 18:22:02 +0200 Jérémy Zurcher \
> > > > > > > > > > <jeremy@asynk.ch> said:
> > > > > > > > > > 
> > > > > > > > > > > just to clarify a few points:
> > > > > > > > > > > 
> > > > > > > > > > > - I think the less macro we have in an eo class declaration the \
> > > > > > > > > > > best, actually we have nothing but that extra first parameter \
> > > > > > > > > > > called eo2_o, wich is either an obj_ptr (devs/tasn/eo2) or a \
> > > > > > > > > > > call_ctx (devs/jeyzu/eo2)
> > > > > > > > > > > 
> > > > > > > > > > > this should go away if we use a stack per thread in eo private \
> > > > > > > > > > > code, so we end up with a clean
> > > > > > > > > > > EAPI float times(float f, float t);
> > > > > > > > > > > 
> > > > > > > > > > > - since day 1 break is supported in eo2_do:
> > > > > > > > > > > #define eo2_do(obj_id, ...)
> > > > > > > > > > > do
> > > > > > > > > > > {
> > > > > > > > > > > obj_ptr_or_ctx = eo2_do_start(obj_id);
> > > > > > > > > > > if(!obj_ptr_or_ctx) break;
> > > > > > > > > > > do { __VA_ARGS__ ; } while (0);
> > > > > > > > > > > eo2_do_end(obj_ptr_or_ctx);
> > > > > > > > > > > } while (0)
> > > > > > > > > > 
> > > > > > > > > > i'm worried about people doing return there. seriously - objid \
> > > > > > > > > > came in becau se of experience that people using efl are in \
> > > > > > > > > > general inexperienced programmers who don't take the time to do \
> > > > > > > > > > things right. they do things quickly and take shortcuts, and they \
> > > > > > > > > > ignore warnings. they'd rather patch out abort()s in efl code \
> > > > > > > > > > forcing them to fix their bugs, than fix their bugs. i am fearful \
> > > > > > > > > > that they will stuff in returns quite happily and think it mostly \
> > > > > > > > > > works most of the time... and then find subtle issues and waste \
> > > > > > > > > > our time finding them. 
> > > > > > > > > > how do we protect/stop returns (or goto's for that matter) within \
> > > > > > > > > > the while block. i looked for some pragmas - can't find any to do \
> > > > > > > > > > this. this would be a really useful compiler feature though (to \
> > > > > > > > > > maybe disable some constructs for a sequence of code).
> > > > > > 
> > > > > > What you seem to be looking for is the cleanup attribute.
> > > > > > 
> > > > > > #define eo2_do(obj_id, ...)
> > > > > > do
> > > > > > {
> > > > > > obj_ptr_or_ctx = eo2_do_start(obj_id);
> > > > > > if(!obj_ptr_or_ctx) break;
> > > > > > do
> > > > > > {
> > > > > > obj_ptr_or_ctx_type  __attribute__((cleanup(eo2_do_end))
> > > > > > dummy = obj_ptr_or_ctx;
> > > > > > __VA_ARGS__ ;
> > > > > > } while (0);
> > > > > > } while (0);
> > > > > > 
> > > > > > 
> > > > > > But then we need to take a look if the cleanup function will run when
> > > > > > the actual function returns, or when the second "do" runs out of
> > > > > > scope.  This attribute is more commonly used to call free on the
> > > > > > variable, so it doesn't matter much.... but for us this would make a
> > > > > > difference if it involves locking.
> > > > > > 
> > > > > > Then you just allow break and return, and the right thing will happen,
> > > > > > even in those cases.
> > > > > 
> > > > > voila! that would do it (if it does work on return as well as break and any
> > > > > goto that jumps out of the while scope). if course it'd be dependant on
> > > > > compiler supporting it - if it doesnt, then we cleanup by hand as normal on \
> > > > > a break and return/goto just create bugs. i'd be ok with that. need to add
> > > > > -fexceptions maybe too from a quick read. needs a little experimenting and \
> > > > > some method of detection. looks like its single parameter only and i guess \
> > > > > it is done variable by variable which is good enough for us. :) i wonder \
> > > > > how new it is. hmm looks like gcc 3.3 - that means it's rather old by now. \
> > > > > GOOD. i hope clang supports it too and.... it seems not. :( oh well. let's \
> > > > > hope most devs still use gcc. :)
> > > > > 
> > > > 
> > > > nice one,
> > > > implemented and tested with gcc 4.8.1 and clang 3.3
> > > > 
> > > > http://git.enlightenment.org/core/efl.git/commit/?h=devs/tasn/eo2&id=275280c3e0fb74e01ffd682acfb69f6a2700dc40
> > > > 
> > > 
> > > Humn... taking what you committed:
> > > 
> > > // eo object method calls batch,
> > > // DO NOT use return statement in it, use break if necessary
> > > #define eo2_do(objid, ...) \
> > > do \
> > > { \
> > > - Eo *_objid_ = objid; \
> > > + Eo *_objid_ EO2_DO_CLEANUP = objid; \
> > > if (!eo2_do_start(_objid_, EINA_FALSE)) break; \
> > > - do { __VA_ARGS__ ; } while (0); \
> > > - eo2_do_end(); \
> > > + __VA_ARGS__; \
> > > 
> > > you still need to stuff the __VA_ARGS__ into a do { } while (0).
> > > Otherwise you are calling eo2_do_end() when eo2_do_start() failed.
> > thanks a lot, I forgot about that !
> > as cleanup is related to variable scope, __VA_ARGS__ is not part of the story,
> > just move _objid_ down to fix, checked with clang and gcc
> > http://git.enlightenment.org/core/efl.git/commit/?h=devs/tasn/eo2&id=a4818d13150114ed0909014c658996be07cf272a
> > 
> 
> 
> It's better to create a new scope. What Lucas meant is that you need to 
> put the __VA_ARGS__ in a sub-scope and then move the variable creation 
> in the same scope, shuffling things around. I'm committing what he meant 
> which I also find cleaner.
> 
> I think that what you do is something that might break in a future 
> version and something that is probably not promised in this extension's 
> documentation.
I was lazy, sure it will be more solid with an added scope
> 
> --
> Tom.
> 
> 
> 
> ------------------------------------------------------------------------------
> See everything from the browser to the database with AppDynamics
> Get end-to-end visibility with application monitoring from AppDynamics
> Isolate bottlenecks and diagnose root cause in seconds.
> Start your free trial of AppDynamics Pro today!
> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
--- Hell'O from Yverdoom

Jérémy (jeyzu)

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic