Closed RazvanN7 closed 3 years ago
Test suite is failing with "All tests have passed".
@WebFreak001 @maxhaton I gave it another pass. Kindly asking for a review
@RazvanN7 Don't have time to consider this thoroughly but looks good to me for now. Thanks.
@WebFreak001 I agree that keeping all the state in one place is easier to maintain, however, at this point that might be an overkill. Anyway, I would rather merge this ASAP to unblock the phobos PRs and refactor later if needed.
travis fail unrelated
can't merge because travis is a required check
There seems to be a dependency error on stdx.allocator:
../subprojects/stdx-allocator/source/stdx/allocator/internal.d(722): Error: static assert: `!__traits(compiles, emplace(& s2, 1))` is false
How do we get passed that? Did I mention that this PR is needed to unblock phobos :-" ?
Ok, so the problem seems to be that libdparse is using an ancient version of stdx.allocator (2.77.5), whereas newer versions don't even contain the assert that is tripping. How do we fix this? Should travis-ci be upgraded somehow or does it suffice to simply change the tag in submodules/stdx-allocator.wrap
[1]? I'm gonna try this.
Edit: Hmm, do I have to specifically mention all recursive dependencies? It looks like the problem is that meson does not know how to pull mir-core (that the newer version of stdx.allocator requires).
[1] https://github.com/dlang-community/libdparse/blob/master/subprojects/stdx-allocator.wrap
@WebFreak001 I have disabled the meson build as it seemed redundant. Once the dependency issues for meson are fixed, the tests can be un-commented. Is that acceptable?
Merging #441 (469bffd) into master (6f0893f) will increase coverage by
0.25%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #441 +/- ##
==========================================
+ Coverage 81.86% 82.12% +0.25%
==========================================
Files 11 11
Lines 8454 8463 +9
==========================================
+ Hits 6921 6950 +29
+ Misses 1533 1513 -20
Impacted Files | Coverage Δ | |
---|---|---|
src/dparse/ast.d | 70.78% <100.00%> (+0.05%) |
:arrow_up: |
src/dparse/parser.d | 91.24% <100.00%> (+0.04%) |
:arrow_up: |
src/dparse/formatter.d | 42.83% <0.00%> (+1.25%) |
:arrow_up: |
src/std/experimental/lexer.d | 86.95% <0.00%> (+2.17%) |
:arrow_up: |
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 6f0893f...469bffd. Read the comment docs.
Is this good to go?
As per: https://github.com/dlang/dmd/pull/11846
A few notes:
AliasAssign
that is viewed as a declaration so that it could appear in the same context as declarations. This might be a bit hacky, but it seemed like the easiest way to implement this.AliasAssign
class is just a wrapper forAliasInitializer
. I considered using anAliasInitializer
as that would make the diff smaller, however, that comes with 2 drawbacks: comments cannot be attached toAliasInitializer
s and it usesAliasInitializer
for a purposes that it wasn't designed for. If the maintainers come to the conclusion that it is better to useAliasInitializer
, I am open to amend the PR.parseDeclaration
, so that is signaled to the declaration.This is currently blocking some PRs in phobos, so if you merge, please also release a new tag so that we can unblock those PRs.
Cheers, RazvanN