date:20160419

[gwt-contrib] Re: dev tests failing locally?

2016-04-19 Thread Colin Alworth

Confirmed, isPublic is true in all three cases, as expected, without the 
old code in place.

On Tuesday, April 19, 2016 at 9:20:10 AM UTC-5, Colin Alworth wrote:
>
> There is no MOD_DEFAULT, at least not yet. I initially attempted to add 
> one and tried to copy the same date from org.objectweb.asm, but their 
> Opcodes type doesn't really have a field that seems to match (no 
> ACC_DEFAULT, etc). If memory serves, bytecode doesn't hold this 
> information, but just has the method marked as not-abstract, and the change 
> to support default methods was in the java lang itself, not the jvm.
>
> This works as expected - here is the test I had stubbed in to confirm that 
> not setting MOD_ABSTRACT still results in it being set (via mapBits):
> public interface Java8Interface {
>   default int defaultImplMethod() {
> return 1;
>   }
>   static int staticImplMethod() {
> return 1;
>   }
>   int noImplMethod();
> }
>
> ...
>   JMethod method = type.getMethod("defaultImplMethod", noParamTypes);
>   assertSame(JPrimitiveType.INT, method.getReturnType());
>   assertEquals(0, method.getParameters().length);
>   assertFalse(method.isStatic());
>   *assertFalse**(method.isAbstract());*
>
>
>   method = type.getMethod("staticImplMethod", noParamTypes);
>   assertSame(JPrimitiveType.INT, method.getReturnType());
>   assertEquals(0, method.getParameters().length);
>   assertTrue(method.isStatic());
>   *assertFalse**(method.isAbstract());*
>
>
>   method = type.getMethod("noImplMethod", noParamTypes);
>   assertSame(JPrimitiveType.INT, method.getReturnType());
>   assertEquals(0, method.getParameters().length);
>   assertFalse(method.isStatic());
>   *assertTrue**(method.isAbstract());*
>
> I've not yet tested public to ensure that it is sane (though was hoping 
> that other unit tests would be unhappy if it wasn't). Before I submit the 
> change, I will check this as well, but the test is currently passing.
>
> On Tuesday, April 19, 2016 at 9:10:05 AM UTC-5, Thomas Broyer wrote:
>>
>>
>>
>> On Tuesday, April 19, 2016 at 2:37:03 PM UTC+2, Colin Alworth wrote:
>>>
>>> As an aside, the fix for the original bug seems to be very easy - the 
>>> code that is building the JMethod instances seems to be overzealous about 
>>> what modifiers must be applied. Aside from the constant failures, all 
>>> compiler tests pass with this change:
>>>
>>> diff --git a/dev/core/src/com/google/gwt/dev/javac/
>>> CompilationUnitTypeOracleUpdater.java b/dev/core/src/com/google/gwt/dev/
>>> javac/CompilationUnitTypeOracleUpdater.java
>>>  
>>> index f63e037..c926ec3 100644 
>>> --- a/dev/core/src/com/google/gwt/dev/javac/
>>> CompilationUnitTypeOracleUpdater.java 
>>> +++ b/dev/core/src/com/google/gwt/dev/javac/
>>> CompilationUnitTypeOracleUpdater.java 
>>> @@ -1121,10 +1121,6 @@ public class CompilationUnitTypeOracleUpdater 
>>> extends TypeOracleUpdater { 
>>>  } 
>>>   
>>>  addModifierBits(method, mapBits(ASM_TO_SHARED_MODIFIERS, methodData
>>> .getAccess())); 
>>> -if (unresolvedType.isInterface() != null) { 
>>> -  // Always add implicit modifiers on interface methods. 
>>> -  addModifierBits(method, Shared.MOD_PUBLIC | Shared.MOD_ABSTRACT); 
>>> -} 
>>>   
>>>  if ((methodData.getAccess() & Opcodes.ACC_VARARGS) != 0) { 
>>>setVarArgs(method);
>>>
>>> With a little more testing, I'll submit this for review, along with a 
>>> test that verifies that it has the desired behavior.
>>>
>>
>> I think this change is overzealous ;-)
>>
>> > Every method declaration in the body of an interface is implicitly 
>> public (§6.6). It is permitted, but discouraged as a matter of style, to 
>> redundantly specify the public modifier for a method declaration in an 
>> interface.
>> > […]
>> > An interface method lacking a default modifier or a static modifier is 
>> implicitly abstract, so its body is represented by a semicolon, not a 
>> block. It is permitted, but discouraged as a matter of style, to 
>> redundantly specify the abstract modifier for such a method declaration.
>> — Source: 
>> https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.4
>>
>> So I think, it should unconditionally add the MOD_PUBLIC bit, and 
>> conditionally add the MOD_ABSTRACT bit if neither MOD_STATIC nor 
>> MOD_DEFAULT is present.
>>
>>
>>

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/cf930d6e-67b3-49f2-9b23-f5b78ce76cf1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[gwt-contrib] Re: dev tests failing locally?

2016-04-19 Thread Colin Alworth

There is no MOD_DEFAULT, at least not yet. I initially attempted to add one 
and tried to copy the same date from org.objectweb.asm, but their Opcodes 
type doesn't really have a field that seems to match (no ACC_DEFAULT, etc). 
If memory serves, bytecode doesn't hold this information, but just has the 
method marked as not-abstract, and the change to support default methods 
was in the java lang itself, not the jvm.

This works as expected - here is the test I had stubbed in to confirm that 
not setting MOD_ABSTRACT still results in it being set (via mapBits):
public interface Java8Interface {
  default int defaultImplMethod() {
return 1;
  }
  static int staticImplMethod() {
return 1;
  }
  int noImplMethod();
}

...
  JMethod method = type.getMethod("defaultImplMethod", noParamTypes);
  assertSame(JPrimitiveType.INT, method.getReturnType());
  assertEquals(0, method.getParameters().length);
  assertFalse(method.isStatic());
  *assertFalse**(method.isAbstract());*


  method = type.getMethod("staticImplMethod", noParamTypes);
  assertSame(JPrimitiveType.INT, method.getReturnType());
  assertEquals(0, method.getParameters().length);
  assertTrue(method.isStatic());
  *assertFalse**(method.isAbstract());*


  method = type.getMethod("noImplMethod", noParamTypes);
  assertSame(JPrimitiveType.INT, method.getReturnType());
  assertEquals(0, method.getParameters().length);
  assertFalse(method.isStatic());
  *assertTrue**(method.isAbstract());*

I've not yet tested public to ensure that it is sane (though was hoping 
that other unit tests would be unhappy if it wasn't). Before I submit the 
change, I will check this as well, but the test is currently passing.

On Tuesday, April 19, 2016 at 9:10:05 AM UTC-5, Thomas Broyer wrote:
>
>
>
> On Tuesday, April 19, 2016 at 2:37:03 PM UTC+2, Colin Alworth wrote:
>>
>> As an aside, the fix for the original bug seems to be very easy - the 
>> code that is building the JMethod instances seems to be overzealous about 
>> what modifiers must be applied. Aside from the constant failures, all 
>> compiler tests pass with this change:
>>
>> diff --git a/dev/core/src/com/google/gwt/dev/javac/
>> CompilationUnitTypeOracleUpdater.java b/dev/core/src/com/google/gwt/dev/
>> javac/CompilationUnitTypeOracleUpdater.java
>>  
>> index f63e037..c926ec3 100644 
>> --- a/dev/core/src/com/google/gwt/dev/javac/
>> CompilationUnitTypeOracleUpdater.java 
>> +++ b/dev/core/src/com/google/gwt/dev/javac/
>> CompilationUnitTypeOracleUpdater.java 
>> @@ -1121,10 +1121,6 @@ public class CompilationUnitTypeOracleUpdater 
>> extends TypeOracleUpdater { 
>>  } 
>>   
>>  addModifierBits(method, mapBits(ASM_TO_SHARED_MODIFIERS, methodData.
>> getAccess())); 
>> -if (unresolvedType.isInterface() != null) { 
>> -  // Always add implicit modifiers on interface methods. 
>> -  addModifierBits(method, Shared.MOD_PUBLIC | Shared.MOD_ABSTRACT); 
>> -} 
>>   
>>  if ((methodData.getAccess() & Opcodes.ACC_VARARGS) != 0) { 
>>setVarArgs(method);
>>
>> With a little more testing, I'll submit this for review, along with a 
>> test that verifies that it has the desired behavior.
>>
>
> I think this change is overzealous ;-)
>
> > Every method declaration in the body of an interface is implicitly 
> public (§6.6). It is permitted, but discouraged as a matter of style, to 
> redundantly specify the public modifier for a method declaration in an 
> interface.
> > […]
> > An interface method lacking a default modifier or a static modifier is 
> implicitly abstract, so its body is represented by a semicolon, not a 
> block. It is permitted, but discouraged as a matter of style, to 
> redundantly specify the abstract modifier for such a method declaration.
> — Source: 
> https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.4
>
> So I think, it should unconditionally add the MOD_PUBLIC bit, and 
> conditionally add the MOD_ABSTRACT bit if neither MOD_STATIC nor 
> MOD_DEFAULT is present.
>
>
>

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/e7f1dbbc-a90a-482f-9919-a02f4d22ef92%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[gwt-contrib] Re: dev tests failing locally?

2016-04-19 Thread Thomas Broyer



On Tuesday, April 19, 2016 at 2:37:03 PM UTC+2, Colin Alworth wrote:
>
> As an aside, the fix for the original bug seems to be very easy - the code 
> that is building the JMethod instances seems to be overzealous about what 
> modifiers must be applied. Aside from the constant failures, all compiler 
> tests pass with this change:
>
> diff --git a/dev/core/src/com/google/gwt/dev/javac/
> CompilationUnitTypeOracleUpdater.java b/dev/core/src/com/google/gwt/dev/
> javac/CompilationUnitTypeOracleUpdater.java
>  
> index f63e037..c926ec3 100644 
> --- a/dev/core/src/com/google/gwt/dev/javac/
> CompilationUnitTypeOracleUpdater.java 
> +++ b/dev/core/src/com/google/gwt/dev/javac/
> CompilationUnitTypeOracleUpdater.java 
> @@ -1121,10 +1121,6 @@ public class CompilationUnitTypeOracleUpdater 
> extends TypeOracleUpdater { 
>  } 
>   
>  addModifierBits(method, mapBits(ASM_TO_SHARED_MODIFIERS, methodData.
> getAccess())); 
> -if (unresolvedType.isInterface() != null) { 
> -  // Always add implicit modifiers on interface methods. 
> -  addModifierBits(method, Shared.MOD_PUBLIC | Shared.MOD_ABSTRACT); 
> -} 
>   
>  if ((methodData.getAccess() & Opcodes.ACC_VARARGS) != 0) { 
>setVarArgs(method);
>
> With a little more testing, I'll submit this for review, along with a test 
> that verifies that it has the desired behavior.
>

I think this change is overzealous ;-)

> Every method declaration in the body of an interface is implicitly public 
(§6.6). It is permitted, but discouraged as a matter of style, to 
redundantly specify the public modifier for a method declaration in an 
interface.
> […]
> An interface method lacking a default modifier or a static modifier is 
implicitly abstract, so its body is represented by a semicolon, not a 
block. It is permitted, but discouraged as a matter of style, to 
redundantly specify the abstract modifier for such a method declaration.
— 
Source: https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.4

So I think, it should unconditionally add the MOD_PUBLIC bit, and 
conditionally add the MOD_ABSTRACT bit if neither MOD_STATIC nor 
MOD_DEFAULT is present.


-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/1e6d2149-0be8-4dd4-ad2a-95841bcace14%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[gwt-contrib] Re: dev tests failing locally?

2016-04-19 Thread Colin Alworth

As an aside, the fix for the original bug seems to be very easy - the code 
that is building the JMethod instances seems to be overzealous about what 
modifiers must be applied. Aside from the constant failures, all compiler 
tests pass with this change:

diff --git a/dev/core/src/com/google/gwt/dev/javac/
CompilationUnitTypeOracleUpdater.java b/dev/core/src/com/google/gwt/dev/
javac/CompilationUnitTypeOracleUpdater.java
 
index f63e037..c926ec3 100644 
--- a/dev/core/src/com/google/gwt/dev/javac/CompilationUnitTypeOracleUpdater
.java 
+++ b/dev/core/src/com/google/gwt/dev/javac/CompilationUnitTypeOracleUpdater
.java 
@@ -1121,10 +1121,6 @@ public class CompilationUnitTypeOracleUpdater extends 
TypeOracleUpdater { 
 } 
  
 addModifierBits(method, mapBits(ASM_TO_SHARED_MODIFIERS, methodData.
getAccess())); 
-if (unresolvedType.isInterface() != null) { 
-  // Always add implicit modifiers on interface methods. 
-  addModifierBits(method, Shared.MOD_PUBLIC | Shared.MOD_ABSTRACT); 
-} 
  
 if ((methodData.getAccess() & Opcodes.ACC_VARARGS) != 0) { 
   setVarArgs(method);

With a little more testing, I'll submit this for review, along with a test 
that verifies that it has the desired behavior.

-- 
You received this message because you are subscribed to the Google Groups "GWT 
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/google-web-toolkit-contributors/f687c323-35af-418e-804d-961d78b8a042%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[gwt-contrib] Re: dev tests failing locally?

[gwt-contrib] Re: dev tests failing locally?

[gwt-contrib] Re: dev tests failing locally?

[gwt-contrib] Re: dev tests failing locally?

4 matches

Site Navigation

Mail list logo

Footer information