Fix issue 578 #591
Merged
Fix issue 578 #591
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Overview:
Array astInformation.structInitEndLocations is used to find index to use
in astInformation.indentInfoSortedByEndLocation.
Both are sorted, first is used to find the array index, second is
accessed at that index to get brace indentation information.
Problem:
structInitEndLocations is generated from struct initializers
exclusively while the brace information array also contains entries
for function literal initializers. Thus when function literal init(s)
are used, we get accumulating off by one errors in the second array:
match value in structInitEndLocations and take that index:
[3, 50]
^--> index 1
take brace indent information from that index:
[3, 15, 50]
| ^--> the one we want
^------> the one we get (function literal init)
Solution:
This guarantees that searching forward works.
While better search strategies than linear are possible, this should be
enough for any sane and most of the insane code files.
I have formatted them both to look like the the individual bad output and not like the actual output from formatting the issue0432.d file with the current implementation. The current implementation makes it look like a new / different problem.
Using different variable to iterate to guarantee unchanged behaviour if anything doesn't work as intended.
Error description: In formatColon() the colon that should match a question mark gets selected as the default case of the if-else ladder which is a guaranteed " : ". Amongst the previous one there is the associated array matcher which is selected since this is happening in an aa (it just checks for surrounding []). Solution: Harden associated array if-condition. Existing data does not contain enough information, store ternary conditional colon indices in an array in ASTInformation. Before an associated array is matched, confirm that it isn't part of ternary conditional. If it is a ternary colon, it will fall through the if-ladder like before.
WebFreak001
reviewed
Oct 22, 2023
tests/allman/issue0578.d.ref
Outdated
| auto aa1 = [0: true ? 1 : 0]; | ||
| auto aa2 = [0: true ? (false ? 1 : 2) : 3]; | ||
|
|
||
| auto aa3 = [0: true ? false ? 1: 2 : 3]; |
There was a problem hiding this comment.
this seems to be a part of the issue you said would be fixed in the title, no?
Still got broken formatting here
There was a problem hiding this comment.
My bad I misunderstood libdparse, I fix it
WebFreak001
approved these changes
Oct 22, 2023
|
please don't work on the master branch locally anymore, create branches for your fixes. You will have problems updating your master branch with upstream now unless you force push or recreate your fork |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The infos are in the commit message, a bit odd is the position of the new array in
ASTInformation(all othersize_t[]arrays come before theBraceIndentInfoandStructInitializerInfoarray). The idea is to prevent breaking any intializers someone may have if this is used as an import. I think that's unlikely though, let me know if I should order it by type.Github tells me this merges 5 commits, but it should only be the last one Fix issue 578. I am not too familiar with github, if I am doing it wrong please let me know.