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

If..Else Inside A While Scope #419

Open
yumamur opened this issue Aug 4, 2023 · 6 comments
Open

If..Else Inside A While Scope #419

yumamur opened this issue Aug 4, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@yumamur
Copy link

yumamur commented Aug 4, 2023

When an if statement exists in a while scope, which is not delimited by curly brackets, Norminette expects the else of the if to be ordinated with the while.

Erroneous code

int	main(void)
{
	while (1)
		if (1)
			break ;
	else
		return (0);
	while (1)
		if (1)
			break ;
		else
			return (0);
}
@matthieu42Network matthieu42Network added the bug Something isn't working label Aug 8, 2023
@kiokuless
Copy link
Contributor

kiokuless commented Aug 12, 2023

Hi @yumamur! You should check the following link. This is not a bug, an intended behavior. If you use an if statement in a while statement, it is obvious that the while statement contains more than two lines.

norminette/pdf/en.norm.tex

Lines 219 to 220 in af5bf14

\item Control structures (if, while..) must have braces, unless they contain a single
line.

@yumamur
Copy link
Author

yumamur commented Aug 14, 2023

Hi @kiokuless. Thanks for the addition, I reported the error unaware of this fact.
But then, even if that is the case, norminette should report the while structure containing more than one line, right?
And we can do something like this, too:

// ...
while (1)
    if (1)
        while (1)
            if (1)
                return (1);
// ...

@kiokuless
Copy link
Contributor

kiokuless commented Aug 14, 2023

Yes, your opinion is better.

But then, even if that is the case, norminette should report the while structure containing more than one line, right?

By the way, I guess it looks like a lot of work to implement this with the current norminette. Because the current norminette implementation is looking at the token sequence by lexical analysis, and doesn't do syntactic analysis. So it seems to be difficult to detect this...

@NiumXp
Copy link
Contributor

NiumXp commented Aug 14, 2023

I guess it looks like a lot of work to implement this with the current norminette.

I believe we can solve this problem just by viewing the history, if one statement (flow control) is inside another one, we show the error. But, history is extremely limited to do this, we just store strings in history instead of Rule instances.

Example:

class CheckStatements(Rule):
    # New design
    def __init__(self, context, scope):
        self.context = context
        self.scope = scope

    def run(self, context):  # Context to compatibility
        if not self.is_statement(context):
            return
        if last := context.history.last:
            if last.scope != self.scope:
                context.new_error("MORE_THAN_1_LINE", context.peek_token(0))

    def is_statement(self, context):
        ...

@kiokuless
Copy link
Contributor

kiokuless commented Aug 14, 2023

I came up with a simpler implementation. It is to check the first token of the statement part of if/if-else/while statements.

  • LBRACE({) is fine as a multiline statement. Need to look at inner statements recursively?
  • "if"/"while" is definitely a multiline statement, then norm will print an error.
  • Otherwise, it is treated as a single-line statement.

Since the control structures allowed in norminette are only if/if-else/while, this approach looks good to me. Is there anything that could be left out?

@NiumXp
Copy link
Contributor

NiumXp commented Aug 14, 2023

@kiokuless, I believe that the problem cannot be solved with lexical analysis (checking if the first token after the conditional of an if, while, etc.) because it is necessary to look where the statement is and not what is in it. So some syntactic analysis would help here: if it's in a BlockStatement then ok, otherwise error.

With your solution, how do you plan to allow the first if and disallow the second in the code below?

void main(void)
{
        if (1)
            return 1;
        if (1)
            if (2)
                return 2;
}

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

4 participants