Skip to content

Conversation

volsa
Copy link
Member

@volsa volsa commented Mar 8, 2024

Resolves #1136

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.75%. Comparing base (b3e27f2) to head (89ef326).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1137   +/-   ##
=======================================
  Coverage   95.74%   95.75%           
=======================================
  Files         147      147           
  Lines       43286    43305   +19     
=======================================
+ Hits        41446    41465   +19     
  Misses       1840     1840           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@volsa volsa requested review from ghaith and mhasel March 8, 2024 08:44
for block in &statement.blocks {
let kind = context.annotations.get_type_or_void(&block.condition, context.index);

if !kind.get_type_information().is_bool() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that this is wrong, but i'm interested to see how often people also use integers here the C way..
I would just accept this for now and and we discuss later if it might need to become a warning. In which case it will need its own error code btw.
Also note to self, when merging the diagnostics make sure this becomes E094

error: Expected a boolean, got `DINT`
┌─ <internal>:12:21
12 │ ELSIF 0 THEN
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think IF 1 and IF 0 are fine.. not because i like them but because 0 and 1 are common I think

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially also wanted to support them but I thought if someone is using 0 or 1 they could also just use FALSE or TRUE instead. But also I just replicated what Rust does, though comparing with C makes more sense generally speaking. Anyhow, do we proceed with supporting 0 and 1s here?

@volsa
Copy link
Member Author

volsa commented Mar 8, 2024

Closing this in favor of #1140

@volsa volsa closed this Mar 8, 2024
@volsa volsa deleted the validate-if branch July 10, 2025 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate if statement
2 participants