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

Warning "Left width does not match right width" should not be reported if width is unknown #1320

Open
cmnrd opened this issue Aug 5, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@cmnrd
Copy link
Collaborator

cmnrd commented Aug 5, 2022

In C++, width can be given as code blocks. In the following example, lfc reports "Left width 25 does not match right width 0" although the widths actually do match.

reactor Foo(dimension: size_t(5)){
    output[dimension] out: void
}

reactor Bar(dimension: size_t(5)) {
    foo = new[dimension] Foo();
    output[{=dimension*dimension=}] out: void;
    foo.out -> out;
}

If we cannot statically decide whether the widths match and the target supports dynamic widths, then we should not report a warning in lfc but leave it to the runtime to check the widths during execution.

@cmnrd cmnrd added the bug Something isn't working label Aug 5, 2022
@petervdonovan
Copy link
Collaborator

Will this become less of an issue if/when we implement some of the ideas in #544? In particular I am referring to AddExpr and MulExpr.

I understand why targets other than the C target are currently more dynamic in that their executables take parameters on the command line, and I also realize that it is complicated to define what arithmetic means in LF (e.g., due to overflow). However, in the long term, I'm not sure I have a good grasp of a) the extent to which properties like port width should be dynamic, and b) the extent to which we want target language code to be used outside reaction bodies.

I have no objections about eliminating the warning, though -- I just wanted to understand the direction we're taking.

@cmnrd
Copy link
Collaborator Author

cmnrd commented Sep 7, 2022

#544 is purely syntactical. It would help in eliminating the {= and =} here but not solve the underlying issue. To solve the issue we would either need to interpret the expression in our LF compiler (which is a rabbit whole of its own and it seems immensely complex in particular considering that different targets use different expression syntax).

Even if we understand some expressions, there are limits. For instance, we could replace dimension*dimension above with a function call. This produces valid C++ code, but we cannot statically decide whether the width match. This is left for the runtime to check. If the width do not match, it produces an error message and aborts execution.

My proposal is not to remove the warning altogether, but to only produce the warning when we statically know that there is a mismatch. In the above example the width of {=dimension*dimension=} is inferred to be 0 for some reason. But it should probably be some invalid value like null or -1 instead. Then we can distinguish whether we actually do know the width or not.

However, in the long term, I'm not sure I have a good grasp of a) the extent to which properties like port width should be dynamic, and b) the extent to which we want target language code to be used outside reaction bodies.

Yes, I think it is an important discussion to have. Generally I am in favor of embracing dynamicity here and accepting that some checks can only be performed at runtime. In the end it is a tradeoff between flexibility and static knowledge, and I think its fine for different targets to make different choices here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants