Sound great. Yes, switching it back on at any point should remain a quick fix option because it would simply make the other code act as a backstop. I will take a quick look on Monday to see if I can find any problems in small as-only projects where maybe the order of class definitions in the swf could be important with the current approach. Sounds a bit like a theoretical edge case but you made me think about it. Have a nice weekend.
-Greg [sent from my phone] On 3/09/2016 12:41 PM, "Alex Harui" <aha...@adobe.com> wrote: > > > On 9/2/16, 5:10 PM, "Greg Dove" <greg.d...@gmail.com> wrote: > > >Yes, that was one of the major changes. I moved it alongside the other > >implementation inside ClassDirectiveProcessor and was able to get more > >consistent results. > >I initially commented it out so I could get everything working using the > >implements IEventDispatcher approach - which is the more general approach > >that would work for all cases- in javascript, then added it back for both > >targets as a 2nd output variation in a later commit, and it went to > >ClassDirectorProcessor for swf. > > > >I was seeing 2 issues from the ASCompilationUnit implementation: > >1. It was not always applying where it should have been (so in cases where > >it could have been 'extends EventDispatcher' it would sometimes fall > >through to the implements IEventDispatcher approach in > >ClassDIrectiveProcessor anyway). I suspect this was threading-related, but > >I am not so familiar with threads. > > > >2. Less of an issue, it could also apply an EventDispatcher base class in > >static-only bindable cases, which is unnecessary, this I expect could have > >been more easily fixed. I tried checking with hasModifier in the original > >code, but I think that might not be available yet, iirc that caused an > >error. > > > >So I moved everything to the ClassDirectiveProcessor, which had the other > >implementation of bindable support in any case. Sorry, I thought I had > >been > >clear about that. > > Well, you probably did explain it, but it probably didn't stick in my head > until we hit this last issue and I went looking for the code where I had a > vague memory of hacking a base class. I still don't know the compiler > code so well that it is clear in my head what work should be done where. > It looks like the original attempt to deal with [Bindable] for SWF was > done in ClassDirectiveProcessor, but when I wanted to fix a bug in > FalconJX I decided to fix it by hacking the AST since that drives > everything in FalconJX. The right answer may have been for me to move the > code from ClassDirectiveProcessor into ASCompilationUnit and use the > existing tests for needsEventDispatcher, then maybe we wouldn't have had > the two issues above. > > Anyway, I think we have recorded the possibility that AST hacking might be > a solution if we hit other problems. I'm ok with leaving the code as is > until we hit another problem but feel free to keep digging if you want. > > Thanks, > -Alex > >
