You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@royale.apache.org by GitBox <gi...@apache.org> on 2021/11/08 07:14:37 UTC

[GitHub] [royale-asjs] Harbs opened a new issue #1163: trace no longer stripped out

Harbs opened a new issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163


   Here is a very simple app which uses trace.
   
   ```
   <?xml version="1.0" encoding="utf-8"?>
   <js:Application xmlns:fx="http://ns.adobe.com/mxml/2009"
                   xmlns:js="library://ns.apache.org/royale/basic"
                   applicationComplete="onComplete()">
       <fx:Script>
           <![CDATA[
               private function onComplete():void{
                   trace("foo");
               }
           ]]>
       </fx:Script>
       <js:valuesImpl>
           <js:SimpleCSSValuesImpl />
       </js:valuesImpl>
       <js:initialView>
           <js:View>
               <js:Label text="ID" id="id1" localId="id1" />
           </js:View>
       </js:initialView>
   </js:Application>
   ```
   `Language.trace` has a "debug comment" of `@royaledebug` This is suposed to cause `if(!goog.DEBUG)return;` to be inserted at the top of the function. That line allows the Google Complier to strip out the entire function and any references to it in the app using dead code removal.
   
   It's not actually being removed. Currently, `Language.trace` is output like this:
   
   ```
   /**
    * @royaledebug
    * @nocollapse
    * @param {...} rest
    */
   org.apache.royale.utils.Language.trace = function(rest) {
     rest = rest;if(!goog.DEBUG)return;
     rest = Array.prototype.slice.call(arguments, 0);
     var /** @type {*} */ theConsole;
     theConsole = goog.global.console;
     if (theConsole === undefined) {
       if (typeof(window) !== "undefined") {
         theConsole = window.console;
       } else if (typeof(console) !== "undefined") {
         theConsole = console;
       }
     }
     try {
       if (theConsole && theConsole.log) {
         theConsole.log.apply(theConsole, rest);
       }
     } catch (e) {
     }
   };
   ```
   Notice that `if(!goog.DEBUG)return;` is being inserted, but it's after `rest = rest;` I don't know if that's the reason it's not being stripped or there's some other reason.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] greg-dove edited a comment on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
greg-dove edited a comment on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-983389610


   > I'm curious why `org.apache.royale.utils.Language` does not exist in output, but we have 194 other `org.apache.royale` classes whose names are preserved.
   > 
   > Looking at. the js output of `org.apache.royale.utils.CSSUtils`, I don't see much difference between that and `org.apache.royale.utils.Language`.
   
   Language is the exception in this case. I added @royalesuppressexport some time ago to get this result and it was as it is now. There was a time in between when it wasn't working, but it is again now. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] Harbs commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-982944008


   It also looks like `@nocollapse` is used for getters (setters?) but not for public vars.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] Harbs commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-962894426


   I don't understand why `@nocollapse` is being added to methods at all. My understanding was that it's needed in some cases for accessors, but it seems like methods should be re-nameable.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] Harbs commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-982951244


   I just retested and `trace` is still there in release.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] greg-dove commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
greg-dove commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-983029376


   > I just retested and `trace` is still there in release.
   
   In what way is it there? if you type **org.apache.royale.utils.Language** into console, you should see undefined (because nothing is exported now for Language).
   If not, please make sure you are using a recent build of Language.swc and and of the compiler


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] Harbs commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-983372528


   I'm curious why `org.apache.royale.utils.Language` does not exist in output, but we have 194 other `org.apache.royale` classes whose names are preserved.
   
   Looking at. the js output of `org.apache.royale.utils.CSSUtils`, I don't see much difference between that and `org.apache.royale.utils.Language`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] greg-dove edited a comment on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
greg-dove edited a comment on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-983029376


   > I just retested and `trace` is still there in release.
   
   In what way is it there? if you type **org.apache.royale.utils.Language** into console, you should see undefined (because nothing is exported now for Language).
   If not, please make sure you are using a recent build of Language.swc and and of the compiler.
   
   Note: the above does not mean that it is definitely dead-code-eliminated. It could be renamed and still present. But I am not sure how to easily check that, so thought I would try to do via debugging it in the compiler.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] Harbs commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-981662052


   @greg-dove @joshtynjala any insights?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] greg-dove commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
greg-dove commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-982269731


   I took a look at your example, @Harbs and I believe the original issue with trace is solved with the export suppression fixes. For the nocollapse question, I don't know the answer, but it has been there since the end of June 2020 and was related to Josh's work on the refactor of the output, so I can only assume it was needed at least during part of those changes - Josh may perhaps recall the details of this (but it was some time ago now). Although I don't completely understand what nocollapse does (I *think* it relates to maintaining the original package/class structure), the GCC documentation is very clear that it does not prevent the compiler from doing renaming itself. I think it was more the export that was holding onto Language.trace. Perhaps 'trace' was special-cased in compiler-jx in earlier code to avoid the old '@export' approach... not sure.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] greg-dove commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
greg-dove commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-983389610


   > I'm curious why `org.apache.royale.utils.Language` does not exist in output, but we have 194 other `org.apache.royale` classes whose names are preserved.
   > 
   > Looking at. the js output of `org.apache.royale.utils.CSSUtils`, I don't see much difference between that and `org.apache.royale.utils.Language`.
   
   
   
   > I'm curious why `org.apache.royale.utils.Language` does not exist in output, but we have 194 other `org.apache.royale` classes whose names are preserved.
   > 
   > Looking at. the js output of `org.apache.royale.utils.CSSUtils`, I don't see much difference between that and `org.apache.royale.utils.Language`.
   
   Language is the exception in this case. I added @royalesuppressexport some time ago to get this result and it was as it is now. There was a time in between when it wasn't working, but it is again now. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] greg-dove commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
greg-dove commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-982941128


   @joshtynjala I have not 100% confirmed that it is dead-code-eliminated, but the export suppression alone has left no obvious hint of it still being there in the release build. I can try and debug into the compiler this weekend to see if it is actually already being completely dead-code eliminated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] Harbs commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-962885866


   It seems like it is the `@nocollapse` being inserted that's causing the issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] joshtynjala commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
joshtynjala commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-982933347


   I found that preventing symbol renaming required `@nocollapse` to be added to more fields (not just accessors). I don't remember the exact details, but I vaguely recall that static fields (including methods) needed it.
   
   That being said, I don't know why this would prevent a static method like `Language.trace()` from being removed as dead code.
   
   Potentially, a solution might be to omit `@nocollapse` when `@royaledebug` is present, as a special case.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] Harbs commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-983367686


   Search the minified code for `trace`. You will see two instances:
   1. `y.trace=m()`
   2. `jf.prototype.Ia=function(){y.trace('foo')};`
   
   The content of the trace method is removed, but the function still exists and every case where it's called is not being removed which can add significantly to output.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] greg-dove commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
greg-dove commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-983385464


   > Search the minified code for `trace`. You will see two instances:
   > 
   > 1. `y.trace=m()`
   > 2. `jf.prototype.Ia=function(){y.trace('foo')};`
   > 
   > The content of the trace method is removed, but the function still exists and every case where it's called is not being removed which can add significantly to output.
   
   Got it, thanks. I can look into it this coming weekend.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [royale-asjs] Harbs commented on issue #1163: trace no longer stripped out

Posted by GitBox <gi...@apache.org>.
Harbs commented on issue #1163:
URL: https://github.com/apache/royale-asjs/issues/1163#issuecomment-982951854


   This is the `asconfig` file I used:
   ```
   {
       "config": "royale",
       "compilerOptions": {
           "debug": true,
           "targets": ["JSRoyale"],
           "source-map": true,
           "remove-circulars": true
       },
       
       "files":
       [
           "src/test_project.mxml"
       ]
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@royale.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org