Alex,

These commits where you added caching for base classes in 
ClassDefinitionBase.isInstanceOf() and DefinitionBase.getParent() have broken 
code intelligence in IDEs like Visual Studio Code. While the caching works for 
a single compile from the command line, an IDE will keep the same instance of 
the compiler running for a long time. The cache becomes stale, and it leads to 
compiler errors that don't make sense.

In particular, I ran into errors when multiple projects in my workspace 
referenced the same class. Both projects have the same source-path, so the 
class was from the same file on the file system in both projects. I was getting 
errors that said that values of MySubclass could not be assigned to variables 
typed as MySuperclass. MySubclass extends MySuperclass, obviously. I'm not very 
familiar with this part of the compiler, but it seems that when the compiler 
switches between projects, the cached values may no longer be valid.

I suspect that there would also be compiler errors if the user were to change 
the superclass of an existing class, even if only a single project were open.

I reverted to the old implementation locally, and I confirmed that my issue 
went away.

I understand that this commit is nearly a year old. I wish that I had 
discovered it sooner, but it took so long between the releases of 0.9.2 and 
0.9.4, that I didn't integrate these changes into the VSCode extension until 
recently.

I can commit my revert, if you think that's the best approach. Or maybe you can 
think of a way to know when the cache should be reset. Or maybe you can create 
some kind of option where IDEs can disable the cache.

- Josh

On 2018/03/30 07:08:10, [email protected] wrote: 
> This is an automated email from the ASF dual-hosted git repository.
> 
> aharui pushed a commit to branch develop
> in repository https://gitbox.apache.org/repos/asf/royale-compiler.git
> 
> commit 52509ab663ffda8808eca6598eef59275e6ee37c
> Author: Alex Harui <[email protected]>
> AuthorDate: Fri Mar 30 00:01:44 2018 -0700
> 
>     try caching some other things to speed up the compiler
> ---
>  .../internal/definitions/ClassDefinitionBase.java  | 56 
> ++++++++++++----------
>  .../internal/definitions/DefinitionBase.java       |  8 +++-
>  2 files changed, 39 insertions(+), 25 deletions(-)
> 
> diff --git 
> a/compiler/src/main/java/org/apache/royale/compiler/internal/definitions/ClassDefinitionBase.java
>  
> b/compiler/src/main/java/org/apache/royale/compiler/internal/definitions/ClassDefinitionBase.java
> index de73e9b..de8cfe1 100644
> --- 
> a/compiler/src/main/java/org/apache/royale/compiler/internal/definitions/ClassDefinitionBase.java
> +++ 
> b/compiler/src/main/java/org/apache/royale/compiler/internal/definitions/ClassDefinitionBase.java
> @@ -414,6 +414,8 @@ public abstract class ClassDefinitionBase extends 
> TypeDefinitionBase implements
>          return new InterfaceDefinition.InterfaceIterator(this, project, 
> null);
>      }
>  
> +    private ArrayList<IDefinition> baseDefinitions = null;
> +    
>      @Override
>      public boolean isInstanceOf(final ITypeDefinition type, ICompilerProject 
> project)
>      {
> @@ -423,35 +425,41 @@ public abstract class ClassDefinitionBase extends 
> TypeDefinitionBase implements
>  
>          if (type instanceof IClassDefinition)
>          {
> -            // We're trying to determine whether this class
> -            // is derived from a specified class ('type').
> -            // Iterate the superclass chain looking for 'type'.
> -            Iterator<IClassDefinition> iter = classIterator(project, false);
> -            while (iter.hasNext())
> -            {
> -                IClassDefinition cls = iter.next();
> -                if (cls == type)
> -                    return true;
> -            }
> -            return false;
> +             if (baseDefinitions == null)
> +             {
> +                     baseDefinitions = new ArrayList<IDefinition>();
> +                     
> +                 // We're trying to determine whether this class
> +                 // is derived from a specified class ('type').
> +                 // Iterate the superclass chain looking for 'type'.
> +                 Iterator<IClassDefinition> iter = classIterator(project, 
> false);
> +                 while (iter.hasNext())
> +                 {
> +                     IClassDefinition cls = iter.next();
> +                     baseDefinitions.add(cls);
> +                 }
> +             }
>          }
>          else if (type instanceof IInterfaceDefinition)
>          {
> -            // We're trying to determine whether this class
> -            // implements a specified interface ('type').
> -            // Iterate all of the interfaces that this class implements,
> -            // looking for 'type'.
> -            Iterator<IInterfaceDefinition> iter = interfaceIterator(project);
> -            while (iter.hasNext())
> -            {
> -                IInterfaceDefinition intf = iter.next();
> -                if (intf == type)
> -                    return true;
> -            }
> -            return false;
> +             if (baseDefinitions == null)
> +             {
> +                     baseDefinitions = new ArrayList<IDefinition>();
> +                     
> +                 // We're trying to determine whether this class
> +                 // implements a specified interface ('type').
> +                 // Iterate all of the interfaces that this class implements,
> +                 // looking for 'type'.
> +                 Iterator<IInterfaceDefinition> iter = 
> interfaceIterator(project);
> +                 while (iter.hasNext())
> +                 {
> +                     IInterfaceDefinition intf = iter.next();
> +                     baseDefinitions.add(intf);
> +                 }
> +             }
>          }
>  
> -        return false;
> +     return baseDefinitions.contains(type);
>      }
>  
>      @Override
> diff --git 
> a/compiler/src/main/java/org/apache/royale/compiler/internal/definitions/DefinitionBase.java
>  
> b/compiler/src/main/java/org/apache/royale/compiler/internal/definitions/DefinitionBase.java
> index 61fb92a..0626789 100644
> --- 
> a/compiler/src/main/java/org/apache/royale/compiler/internal/definitions/DefinitionBase.java
> +++ 
> b/compiler/src/main/java/org/apache/royale/compiler/internal/definitions/DefinitionBase.java
> @@ -166,6 +166,8 @@ public abstract class DefinitionBase implements 
> IDocumentableDefinition, IDefini
>  
>      private int absoluteNameStart = 0;
>      private int absoluteNameEnd = 0;
> +    
> +    private IDefinition parentDef = null;
>  
>      /**
>       * Called by {@code MXMLScopeBuilder} when building definitions from
> @@ -240,6 +242,9 @@ public abstract class DefinitionBase implements 
> IDocumentableDefinition, IDefini
>      @Override
>      public IDefinition getParent()
>      {
> +     if (parentDef != null)
> +             return parentDef;
> +     
>          IASScope scope = getContainingScope();
>  
>          // Walk up the scope chain until we find a scope that has
> @@ -251,7 +256,8 @@ public abstract class DefinitionBase implements 
> IDocumentableDefinition, IDefini
>          while ((scope != null) && (scope.getDefinition() == null))
>              scope = scope.getContainingScope();
>  
> -        return scope != null ? scope.getDefinition() : null;
> +        parentDef = scope != null ? scope.getDefinition() : null;
> +        return parentDef;
>      }
>  
>      @Override
> 
> -- 
> To stop receiving notification emails like this one, please contact
> [email protected].
> 

Reply via email to