digitalmars.D - [SAoC2022] Replace libdparse with dmd-as-a-library in D-Scanner
- Lucian Danescu (41/41) Dec 13 2022 Hello!
- rikki cattermole (4/8) Dec 13 2022 Because of string mixins and CTFE I'm not sure this can be 100%.
- Lucian Danescu (6/14) Dec 14 2022 Well, in phobos there are examples with a lot more imbricated
- user1234 (11/30) Dec 14 2022 The dparse-based implementation assumes that the version
- user1234 (6/38) Dec 14 2022 About the second problematic check I think that @property checks
Hello! Since my last update I created 2 pull requests for new visitors: - [auto_func](https://github.com/Dlang-UPB/D-scanner/pull/59) - [backwards_range](https://github.com/Dlang-UPB/D-scanner/pull/58) And did some refactoring for [static_if_else](https://github.com/Dlang-UPB/D-scanner/pull/56) Also created these pull requests in `dmd` adding `isStaticIfCondtion` method, and fixing a minor bug in `TemplateDeclaration` constructor in `ASTBase` - [isStaticIfCondition](https://github.com/dlang/dmd/pull/14689) - [template_constructor](https://github.com/dlang/dmd/pull/14691) I also encountered some difficulties for 2 particular checks: 1. I think I already mentioned this a while back but still I didn't manage to get it straight so I am mentioning this again. [This](https://github.com/Dlang-UPB/D-scanner/blob/replace_libdparse/src/dscanner/analysis/label_va _same_name_check.d) is a visitor that checks if a variable name shadow another variable from an outer scope. The problems come from `static if` and `version` conditions, as these do not introduce a new scope, and for me at least it is not yet 100% clear how to treat them, as things get really complicated when we have many `static if` mixed with `version`. Here is an example of "troublesome" code for this check: ``` void f() { version (Windows) int a; static if (true) version (POSIX) int a; else int b; } ``` My implementation would wrongly issue a warning in this scenario. If anyone would have any suggestions on how to go about this it would be awesome. This is the [pr](https://github.com/Dlang-UPB/D-scanner/pull/41) where I tried a few different implementations for that. Feel free to leave a comment :) 2. Second issue I ran into would be regarding ` property` functions with no arguments that should be marked `const`. In order not to extend this post even more please check [this](https://github.com/dlang/phobos/pull/8648) out for more explanations regarding why that check could be problematic. Thank you!
Dec 13 2022
On 14/12/2022 6:31 AM, Lucian Danescu wrote:The problems come from `static if` and `version` conditions, as these do not introduce a new scope, and for me at least it is not yet 100% clear how to treat them, as things get really complicated when we have many `static if` mixed with `version`.Because of string mixins and CTFE I'm not sure this can be 100%. I'd argue the example code is bad anyway, it could be simplified and it won't be as easy to error.
Dec 13 2022
Lucian Danescu <lucidanescu28 yahoo.com> On Wednesday, 14 December 2022 at 04:38:59 UTC, rikki cattermole wrote:On 14/12/2022 6:31 AM, Lucian Danescu wrote:Well, in phobos there are examples with a lot more imbricated `static if` and `version`. Regarding mixins, true they are ditched for this check. For me it's not really clear how to treat the `static if` and `version` statements.The problems come from `static if` and `version` conditions, as these do not introduce a new scope, and for me at least it is not yet 100% clear how to treat them, as things get really complicated when we have many `static if` mixed with `version`.Because of string mixins and CTFE I'm not sure this can be 100%. I'd argue the example code is bad anyway, it could be simplified and it won't be as easy to error.
Dec 14 2022
On Tuesday, 13 December 2022 at 17:31:46 UTC, Lucian Danescu
wrote:
Hello!
[...]
Here is an example of "troublesome" code for this check:
```
void f()
{
version (Windows)
int a;
static if (true)
version (POSIX)
int a;
else
int b;
}
```
My implementation would wrongly issue a warning in this
scenario. If anyone would have any suggestions on how to go
about this it would be awesome.
The dparse-based implementation assumes that the version
condition are awlays true and in consequence it ignores
everything that's declared in the "else" blocks. You must have
missed something in the DMDFE version of the AST but to be fair
at first glance your translation looks faithful.
But in any case the way it works now is not correct either so I'd
advice to skip entirely static conditions and versions. It will
be possible to handle these two cases the day the semantics will
be run.
Dec 14 2022
On Wednesday, 14 December 2022 at 15:43:53 UTC, user1234 wrote:On Tuesday, 13 December 2022 at 17:31:46 UTC, Lucian Danescu wrote:About the second problematic check I think that property checks can be disabled for now (as you wanted to do) as this attribute is considered by users as almost useless. In both cases the idea is to move forward, just keep track of the problems using issues or code comments.Hello! [...] Here is an example of "troublesome" code for this check: ``` void f() { version (Windows) int a; static if (true) version (POSIX) int a; else int b; } ``` My implementation would wrongly issue a warning in this scenario. If anyone would have any suggestions on how to go about this it would be awesome.The dparse-based implementation assumes that the version condition are awlays true and in consequence it ignores everything that's declared in the "else" blocks. You must have missed something in the DMDFE version of the AST but to be fair at first glance your translation looks faithful. But in any case the way it works now is not correct either so I'd advice to skip entirely static conditions and versions. It will be possible to handle these two cases the day the semantics will be run.
Dec 14 2022