improve fasta unterminated string recovery

improve fasta unterminated string recovery

* enhance handling of unterminated strings
* update ast builder and string validator to handle unterminated strings

[danrubel, 2017-05-29 03:43:43]

danrubel@google.com changed reviewers:
+ ahe@google.com, scheglov@google.com

[ahe, 2017-05-29 15:05:31]

I think I have to give up on this CL for now, and start again tomorrow morning
(or possibly later tonight). Right now, my head is too mushy.

I did spot something that may cause crashes.

https://codereview.chromium.org/2912693002/diff/1/pkg/compiler/lib/src/string...
File pkg/compiler/lib/src/string_validator.dart (right):

https://codereview.chromium.org/2912693002/diff/1/pkg/compiler/lib/src/string...
pkg/compiler/lib/src/string_validator.dart:32: rightQuote =
quoting.rightQuoteLength;
What happens with this string:

"""x"

https://codereview.chromium.org/2912693002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/quote.dart (right):

https://codereview.chromium.org/2912693002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/quote.dart:138: return
unescape(string.substring(startIndex, endIndex), quote);
Same question.

[danrubel, 2017-05-31 13:54:08]

On 2017/05/29 15:05:31, ahe wrote:
> I think I have to give up on this CL for now, and start again tomorrow morning
> (or possibly later tonight). Right now, my head is too mushy.
> 
> I did spot something that may cause crashes.
> 
>
https://codereview.chromium.org/2912693002/diff/1/pkg/compiler/lib/src/string...
> File pkg/compiler/lib/src/string_validator.dart (right):
> 
>
https://codereview.chromium.org/2912693002/diff/1/pkg/compiler/lib/src/string...
> pkg/compiler/lib/src/string_validator.dart:32: rightQuote =
> quoting.rightQuoteLength;
> What happens with this string:
> 
> """x"
> 
>
https://codereview.chromium.org/2912693002/diff/1/pkg/front_end/lib/src/fasta...
> File pkg/front_end/lib/src/fasta/quote.dart (right):
> 
>
https://codereview.chromium.org/2912693002/diff/1/pkg/front_end/lib/src/fasta...
> pkg/front_end/lib/src/fasta/quote.dart:138: return
> unescape(string.substring(startIndex, endIndex), quote);
> Same question.

Glad that you pointed this out. When I dug further into the parser to better
understand the issue, I found that it did not handle sequential string tokens
the way that I thought it would. Per our discussion today, I'll rework this and
have a new CL for you to review later in the week. Thanks!

[danrubel, 2017-05-31 13:54:21]

Description was changed from

==========
improve fasta unterminated string recovery

* enhance handling of unterminated strings
* update ast builder and string validator to handle unterminated strings
==========

to

==========
improve fasta unterminated string recovery

* enhance handling of unterminated strings
* update ast builder and string validator to handle unterminated strings
==========

[danrubel, 2017-06-01 17:11:34]

On 2017/05/31 13:54:08, danrubel wrote:
> On 2017/05/29 15:05:31, ahe wrote:
> > I think I have to give up on this CL for now, and start again tomorrow
morning
> > (or possibly later tonight). Right now, my head is too mushy.
> > 
> > I did spot something that may cause crashes.
> > 
> >
>
https://codereview.chromium.org/2912693002/diff/1/pkg/compiler/lib/src/string...
> > File pkg/compiler/lib/src/string_validator.dart (right):
> > 
> >
>
https://codereview.chromium.org/2912693002/diff/1/pkg/compiler/lib/src/string...
> > pkg/compiler/lib/src/string_validator.dart:32: rightQuote =
> > quoting.rightQuoteLength;
> > What happens with this string:
> > 
> > """x"
> > 
> >
>
https://codereview.chromium.org/2912693002/diff/1/pkg/front_end/lib/src/fasta...
> > File pkg/front_end/lib/src/fasta/quote.dart (right):
> > 
> >
>
https://codereview.chromium.org/2912693002/diff/1/pkg/front_end/lib/src/fasta...
> > pkg/front_end/lib/src/fasta/quote.dart:138: return
> > unescape(string.substring(startIndex, endIndex), quote);
> > Same question.
> 
> Glad that you pointed this out. When I dug further into the parser to better
> understand the issue, I found that it did not handle sequential string tokens
> the way that I thought it would. Per our discussion today, I'll rework this
and
> have a new CL for you to review later in the week. Thanks!

See https://codereview.chromium.org/2915093002/