Add FrontEndStrategy, ResolutionFrontEndStrategy and KernelFrontEndStrategy

Add FrontEndStrategy, ResolutionFrontEndStrategy and KernelFrontEndStrategy

This is a step towards making the compiler agnostic of how the element
model is created. This enables us to build support for the kernel based
front end in parallel with the existing resolver based front end.

R=sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/83e971b315e58307d35ceaa5467d89accb32c4c4

[Johnni Winther, 2017-04-24 13:50:58]

johnniwinther@google.com changed reviewers:
+ sigmund@google.com

[Johnni Winther, 2017-04-24 14:05:42]

Description was changed from

==========
Add FrontEndStrategy, ResolutionFrontEndStrategy and KernelStrategy

This is a step towards making the compiler agnostic of how the element
model is created. This enables us to build support for the kernel based
front end in parallel with the existing resolver based front end.
==========

to

==========
Add FrontEndStrategy, ResolutionFrontEndStrategy and KernelFrontEndStrategy

This is a step towards making the compiler agnostic of how the element
model is created. This enables us to build support for the kernel based
front end in parallel with the existing resolver based front end.
==========

[Siggi Cherem (dart-lang), 2017-04-24 19:36:48]

lgtm, thanks Johnni. A few minor suggestions below.

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/kernel_strategy.dart (right):

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_strategy.dart:19: /// Frontend strategy that
loads '.dill' files and builds a resolved element
nit: for consistency,  maybe split "frontend" as "front end"

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/native...
File pkg/compiler/lib/src/native/resolver.dart (right):

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:34: abstract class AnnotationProcessor
{
maybe this class shouldn't be here? move to builder? or to frontend_strategy?

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:277: checkNativeAnnotation(Compiler
compiler, ClassElement cls,
it appears this is dead code

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/resolu...
File pkg/compiler/lib/src/resolution_strategy.dart (right):

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/resolu...
pkg/compiler/lib/src/resolution_strategy.dart:5: library
dart2js.resolution_strategy;
let's move this file under "resolution/"

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/resolu...
pkg/compiler/lib/src/resolution_strategy.dart:284: /// triggering pre-parsing
and validation of the annotations.
Thanks for adding a comment to this :) - A couple nits:
 - "Original" might be hard to understand to someone new to the codebase, maybe
"AST-based"?
 - "pre-parsing" might be a bit misleading - we do parse it, just not resolve
it? Maybe we can expand this more?

Here is a possible suggestion:

AST-based logic for processing annotations. These annotations are processed very
early in the compilation pipeline, typically this is before resolution is
complete. Because of that this processor does a lightweight parse of the
annotation (which is restricted to a limited subset of the annotation syntax),
and, once resolution completes, it validates that the parsed annotations
correspond to the correct element.

[Johnni Winther, 2017-04-25 07:52:52]

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/kernel_strategy.dart (right):

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/kernel_strategy.dart:19: /// Frontend strategy that
loads '.dill' files and builds a resolved element
On 2017/04/24 19:36:47, Siggi Cherem (dart-lang) wrote:
> nit: for consistency,  maybe split "frontend" as "front end"

Done.

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/native...
File pkg/compiler/lib/src/native/resolver.dart (right):

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:34: abstract class AnnotationProcessor
{
On 2017/04/24 19:36:47, Siggi Cherem (dart-lang) wrote:
> maybe this class shouldn't be here? move to builder? or to frontend_strategy?

Moved to frontend_strategy

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:277: checkNativeAnnotation(Compiler
compiler, ClassElement cls,
On 2017/04/24 19:36:47, Siggi Cherem (dart-lang) wrote:
> it appears this is dead code

Deleted

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/resolu...
File pkg/compiler/lib/src/resolution_strategy.dart (right):

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/resolu...
pkg/compiler/lib/src/resolution_strategy.dart:5: library
dart2js.resolution_strategy;
On 2017/04/24 19:36:48, Siggi Cherem (dart-lang) wrote:
> let's move this file under "resolution/"

Done.

https://codereview.chromium.org/2841583002/diff/1/pkg/compiler/lib/src/resolu...
pkg/compiler/lib/src/resolution_strategy.dart:284: /// triggering pre-parsing
and validation of the annotations.
On 2017/04/24 19:36:48, Siggi Cherem (dart-lang) wrote:
> Thanks for adding a comment to this :) - A couple nits:
>  - "Original" might be hard to understand to someone new to the codebase,
maybe
> "AST-based"?
>  - "pre-parsing" might be a bit misleading - we do parse it, just not resolve
> it? Maybe we can expand this more?
> 
> Here is a possible suggestion:
> 
> AST-based logic for processing annotations. These annotations are processed
very
> early in the compilation pipeline, typically this is before resolution is
> complete. Because of that this processor does a lightweight parse of the
> annotation (which is restricted to a limited subset of the annotation syntax),
> and, once resolution completes, it validates that the parsed annotations
> correspond to the correct element.

Used your suggestion (and Emily is to thank for the original comment)

[Johnni Winther, 2017-04-25 08:07:24]

Description was changed from

==========
Add FrontEndStrategy, ResolutionFrontEndStrategy and KernelFrontEndStrategy

This is a step towards making the compiler agnostic of how the element
model is created. This enables us to build support for the kernel based
front end in parallel with the existing resolver based front end.
==========

to

==========
Add FrontEndStrategy, ResolutionFrontEndStrategy and KernelFrontEndStrategy

This is a step towards making the compiler agnostic of how the element
model is created. This enables us to build support for the kernel based
front end in parallel with the existing resolver based front end.

R=sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/83e971b315e58307d35ceaa5467d89accb32c4c4
==========

[Johnni Winther, 2017-04-25 08:07:25]

Committed patchset #2 (id:20001) manually as
83e971b315e58307d35ceaa5467d89accb32c4c4 (presubmit successful).