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

Enforcement of usage of special variables #404

Open
GreyCat opened this issue Apr 2, 2018 · 6 comments
Open

Enforcement of usage of special variables #404

GreyCat opened this issue Apr 2, 2018 · 6 comments

Comments

@GreyCat
Copy link
Member

GreyCat commented Apr 2, 2018

Follow up for kaitai-io/kaitai_struct_compiler#134.

We have lots of expressions, and we have several special variables, which only make sense in certain special cases.

Currently, we have almost no checking / enforcement of their correct usage. I propose to:

  • draft a list of possible usage places (i.e. if, size, repeat-until, repeat-expr, type invocation parameters, etc),
  • draft a list of variables (there are externally exposed _ and _index, and seemingly internal-only _buf, _on, _is_le — may be we should make some of them public?),
  • do a cross-product of these two lists and think of whether it makes sense to allow usage of variable in these places.
  • enforce this by adding relevant checks and tests

Cc @tschoening

@KOLANICH
Copy link

KOLANICH commented Apr 2, 2018

👎 for that, I vote for supporting nasty incompatible hacks because they may be workarounds for compiler limitations. But detection may be fine. So I propose not to think "what makes sense", but to test for all the supported languages all the combinations and consider everything working everywhere as making sense. But not enforce it for everyone, just emit a warning.

@arekbulski
Copy link
Member

arekbulski commented Apr 2, 2018

👎 as well. There is no benefit to this, and it would sink time and effort to implement it. Well, maybe I am not entirely right. There would be benefit to flagging suspicious schemas/expressions to user (a warning or failure). You have no idea how many times I have seen people coming to Construct forum and posting ridiculous codes (and half of them even worked, which was even more suprising that someone would even write it in the first place). So it makes sense, I grant you that. But I dont think its worth the effort. Not with all the other features in the backlog.

@GreyCat
Copy link
Member Author

GreyCat commented Apr 2, 2018

I want to remind that KS has a few goals like:

  • making things work the same for all languages
  • compiling valid code in target language

So, obviously, we'll need to test lots of combinations, but if it "works" for one target language and does not for the language, my point is that we need to do something about it. Either make it work for all languages, or not work (ideally, with an error in ksc compile time) for all languages.

Addition of this issue does not mean that it's top priority or anything. It's just for the sake of not forgetting that it should be done eventually.

@arekbulski
Copy link
Member

Ah okay, got it. Low priority.

@ams-tschoening
Copy link

+1 for @GreyCat's argument, because everything which works in one language and doesn't in another is simply undefined behaviour, which people try to reduce in all other languages/compilers as well. Additionally, that undefined behaviour leads to PRs like mine in which I think to fix things for the languages I need and ultimately KS might don't want to work that way at all. So people might even invest time for things not wanted by purpose at all, only because they don't know better.

If KSC errors on such things, people need to think twice about their current solution and might find other ways implementing things. In theory, they can even fork and implement custom behaviour for KSC as they see fit anyway.

@arekbulski
Copy link
Member

Oh well, you both made a fine argument. You convinced me. Happy now? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants