trace no longer stripped out

[Harbs]

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.

[Harbs]

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

[Harbs]

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.

[Harbs]

@greg-dove @joshtynjala any insights?

[greg-dove]

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.

[joshtynjala]

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.

[greg-dove]

@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.

[Harbs]

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

[Harbs]

I just retested and trace is still there in release.

[Harbs]

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"
    ]
}

[greg-dove]

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.

[Harbs]

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.

[Harbs]

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.

[greg-dove]

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.

[greg-dove]

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.

[greg-dove]

This issue was transferred from royale-asjs to royale-compiler

[greg-dove]

@joshtynjala I made it so that @royalesuppressexport also avoids emitting @nocollapse which does address the issue raised by @Harbs here for trace. It was keeping hold of an empty function with the @nocollapse. I'll leave it up to you to consider whether or not you think there is more to do and if not, please feel free to close this issue (I don't know if there is more use of @nocollapse than is needed by default, I am pretty sure you know more about this stuff than the rest of us because you 'had your head in it' for some time).

[Harbs]

@greg-dove How did you transfer the issue?

Ah. I see there's an option on the right side. Cool TIL...

[joshtynjala]

@nocollapse is added when something shouldn't be renamed, and exporting is technically different. However, I feel like this is the best solution we have for this issue, at least currently. I'll close this, and revisit if other issues come up.