Prototype method incorrectly removed

[GoogleCodeExporter]

// ==ClosureCompiler==
// @compilation_level ADVANCED_OPTIMIZATIONS
// @output_file_name default.js
// @formatting pretty_print
// ==/ClosureCompiler==

/** @const */
var foo = {};
foo.bar = {
  'bar1': function() { console.log('bar1'); }
}

/** @constructor */
function foobar() {}
foobar.prototype = foo.bar;

foo.foobar = new foobar;

console.log(foo.foobar['bar1']);

Original issue reported on code.google.com by chadkill...@missouristate.edu on 14 Jun 2013 at 2:29

[GoogleCodeExporter]

While in the above snippet the prototype method is completely removed, the 
actual case I came across was doing prototype devirtualization on the quoted 
prototype property.

Something akin to:

foo.foobar.foobar_prototype$bar1 = true;

Which is also bad

Original comment by chadkill...@missouristate.edu on 14 Jun 2013 at 3:10

[GoogleCodeExporter]

Thanks for the report.

Re: comment #1. Devirtualization only happens with functions.  Disambiguate 
properties will not remove it from the prototype.  It would be good to know 
which case you saw.  Neither should modify quoted properties.

Original comment by concavel...@gmail.com on 18 Jun 2013 at 3:21

• Changed state: Accepted

[GoogleCodeExporter]

Let me try it without ambituate/disambituate turned on and see if it still 
occurs in the original source. If it helps, the original problem is:

Prototype assignment -> 
https://github.com/jquery/sizzle/blob/master/sizzle.js#L1393
Properties affected -> 
https://github.com/jquery/sizzle/blob/master/sizzle.js#L1183

When I tried to create repo cases, I could only get complete removal to occur.

Original comment by chadkill...@missouristate.edu on 18 Jun 2013 at 3:27

[GoogleCodeExporter]

It does appear to be an ambiguate/disambiguate problem.

Would it help if I uploaded the test files?

Original comment by chadkill...@missouristate.edu on 18 Jun 2013 at 8:21

[GoogleCodeExporter]

yes, it would.

Original comment by concavel...@gmail.com on 19 Jun 2013 at 8:22

[GoogleCodeExporter]

Compilation arguments:
--compilation_level ADVANCED_OPTIMIZATIONS --warning_level VERBOSE 
--use_types_for_optimization --externs issue1024externs.js --js 
issue1024code.js --formatting PRETTY_PRINT --debug

Uncompiled line numbers: 1214, 1427
Compiled line number: 682

Two warnings are expected (and unrelated).

Original comment by chadkill...@missouristate.edu on 20 Jun 2013 at 2:25

Attachments:

• issue1024code.js
• issue1024externs.js

[GoogleCodeExporter]

Using:

// ==ClosureCompiler==
// @compilation_level ADVANCED_OPTIMIZATIONS
// @output_file_name default.js
// @code_url 
https://closure-compiler.googlecode.com/issues/attachment?aid=10240006000&name=i
ssue1024code.js&token=DgSPbE2vzBM3gJRt2CuVVwWIAoQ%3A1377294668082
// @externs_url 
https://closure-compiler.googlecode.com/issues/attachment?aid=10240006001&name=i
ssue1024externs.js&token=xPbcWt0s89sOIh7D6-V5QCfrjvE%3A1377294668082
// @use_types_for_optimization true
// @warning_level VERBOSE
// @formatting pretty_print
// @debug true
// ==/ClosureCompiler==

I was able to see: the quoted "not" property renamed as:

setFilters_prototype$not

which is disambiguate properties at work, but I'm not sure if it is confused by 
the type system or is confusing itself.

Original comment by concavel...@gmail.com on 24 Aug 2013 at 2:01

[GoogleCodeExporter]

Note it doesn't change it from a quoted property so it remains 
setFilters_prototype$not in the output.

Original comment by concavel...@gmail.com on 24 Aug 2013 at 2:08

[GoogleCodeExporter]

That's the property behavior I was having. I couldn't figure out what type 
annotations (if any) were causing the issue.

Original comment by chadkill...@missouristate.edu on 24 Aug 2013 at 2:17

[GoogleCodeExporter]

Actually why should type annotations matter? If it's quoted, disambiguate 
properties should bail on renaming. This is probably what's going on in the 
simple case I originally posted as well.

Original comment by chadkill...@missouristate.edu on 24 Aug 2013 at 11:37

[GoogleCodeExporter]

I have a fix pending (a one liner essentially plus some unit tests).   
Basically, I was misunderstanding what you were originally saying.  The clues 
were there but "removed" and "devirtualized" were misleading to me.  The 
property was still there just inappropriately renamed by disambiguate 
properties.     The fix will most likely land on Monday.   Longer term we need 
to add some "isThisAnExternalProperty" method in a calling convention.   It 
isn't really good to have the ALL_UNQUOTED property renaming policy hard coded 
into all our passes.

Original comment by concavel...@gmail.com on 24 Aug 2013 at 10:24

[GoogleCodeExporter]

Thanks John. Sorry about the confusion.

Original comment by chadkill...@missouristate.edu on 26 Aug 2013 at 2:04

[GoogleCodeExporter]

This issue was closed by revision f17961ae4d3d.

Original comment by blic...@google.com on 26 Aug 2013 at 5:01

• Changed state: Fixed

[GoogleCodeExporter]

This issue was closed by revision f17961ae4d3d.

Original comment by blic...@google.com on 16 Oct 2013 at 6:20