Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent invalid scope when parsing bodies containing erroneous decls #359

Merged
merged 1 commit into from May 11, 2019
Merged

Conversation

ghost
Copy link

@ghost ghost commented May 11, 2019

Required to fix ... dlang-community/DCD#600

@ghost ghost added the enhancement label May 11, 2019
@ghost
Copy link
Author

ghost commented May 11, 2019

this breaks the arbitrary lookups required by the language.

@ghost ghost closed this May 11, 2019
@ghost ghost deleted the dcd-600 branch May 11, 2019 19:34
@WebFreak001
Copy link
Member

if parseDeclarationsAndStatements can't return null anymore, I think the following classes/members would never be null anymore:

  • CaseStatement.declarationsAndStatement
  • CaseRangeStatement.declarationsAndStatement
  • DefaultStatement.declarationsAndStatement

BlockStatement.declarationsAndStatement before was null with an error in the content, now it is really only null if it's empty (} following the {)

Because of this PR I also found the following potential null-reference-errors which would be fixed by this:

formatter.d:616 (void format(const CaseRangeStatement caseRangeStatement)) and formatter.d:638 (void format(const CaseStatement caseStatement)) both call formatCaseDecls without null check of declarationsAndStatements and the method itself just accesses members.

formatter.d:1036 (void format(const DefaultStatement defaultStatement)) directly accesses the member without null check

nvm stopping checking this out if it's deleted then

@ghost
Copy link
Author

ghost commented May 11, 2019

Yeah the formatter bugs are not a surprise, see #346.

@ghost ghost restored the dcd-600 branch May 11, 2019 19:44
@ghost
Copy link
Author

ghost commented May 11, 2019

@WebFreak001, I have maybe a working fix coming.

@ghost ghost reopened this May 11, 2019
@codecov
Copy link

codecov bot commented May 11, 2019

Codecov Report

Merging #359 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #359      +/-   ##
==========================================
+ Coverage   74.16%   74.17%   +0.01%     
==========================================
  Files           8        8              
  Lines        6874     6877       +3     
==========================================
+ Hits         5098     5101       +3     
  Misses       1776     1776
Impacted Files Coverage Δ
src/dparse/parser.d 90.77% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61519b6...f773808. Read the comment docs.

@ghost
Copy link
Author

ghost commented May 11, 2019

Okay it seems to work and doesn't break dparse test suite. This time the approach is more pragmatic. The case that breaks DCD, i.e a syntax error, is exactly detected.

src/dparse/parser.d Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented May 11, 2019

DCD test suite works too now.

@ghost ghost added the auto-merge label May 11, 2019
@dlang-bot dlang-bot merged commit ec2089d into dlang-community:master May 11, 2019
@ghost ghost deleted the dcd-600 branch May 12, 2019 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants