Jump to content

Sky Slate Blueberry Blackcurrant Watermelon Strawberry Orange Banana Apple Emerald Chocolate
Photo

Unpaired */ treated as continuation section


  • Please log in to reply
9 replies to this topic
polyethene
  • Members
  • 5519 posts
  • Last active: May 17 2015 06:39 AM
  • Joined: 26 Oct 2012
Found this interesting bug while working on IronAHK (ab82004c):

/* [color=grey]; comment out this line[/color]
MsgBox, first
*/ MsgBox, second

autohotkey.com/net Site Manager

 

Contact me by email (polyethene at autohotkey.net) or message tidbit


Lexikos
  • Administrators
  • 9844 posts
  • AutoHotkey Foundation
  • Last active:
  • Joined: 17 Oct 2006

Method #1: A line that starts with "and", "or", ||, &&, a comma, or a period is automatically merged with the line directly above it (in v1.0.46+, the same is true for all other expression operators except ++ and --).
Source: AutoHotkey Scripts and Macros

Rather than "expression operators", it's really any character or character sequence which forms an operator, whether or not it's actually part of an expression. Continuation sections/lines are handled at a much earlier stage than expressions, and the logic used only excludes hotkeys and hotstrings (such as ,::MsgBox). I'd say it's by design, rather than an outright bug.
MsgBox, first
*** MsgBox, second
MsgBox, first
/// MsgBox, second


polyethene
  • Members
  • 5519 posts
  • Last active: May 17 2015 06:39 AM
  • Joined: 26 Oct 2012
A single line comment character (;) does not form part of a continuation section so it would be more intuitive by design to also ignore */ to give comments precedence above all else.

autohotkey.com/net Site Manager

 

Contact me by email (polyethene at autohotkey.net) or message tidbit


fincs
  • Moderators
  • 1662 posts
  • Last active:
  • Joined: 05 May 2007
Reminds me of this:
; gives syntax error
MsgBox % "Hello ; World!"


RaptorX
  • Members
  • 751 posts
  • Last active: Feb 19 2015 02:47 AM
  • Joined: 19 Feb 2010

Reminds me of this:

; gives syntax error
MsgBox % "Hello ; World!"


well, you are commenting out the closing quote so you are trying to pass
MsgBox % "Hello

which will always give a syntax error.

Jamie
  • Members
  • 129 posts
  • Last active: Dec 02 2012 04:59 AM
  • Joined: 26 Mar 2010
I would have expected */ to generate a syntax error if there is no matching /*. It should not be ignored, it should generate a syntax error.

Treating it as a continuation "by design" is really a stretch. The sequence */ should be treated as a single token, not as two operators. If the parser only feels like looking ahead one character then that's simply a reason for the bug, not a justification for a design.

And ; within a string should not begin a comment. It should be treated as a literal semicolon. I'd call that a bug too. Comment characters and sequences should lose their special function within string literals, just like they do in every other language.

I believe that to defend existing behavior as "correct" through mental contortions does a disservice to AHK, because it inhibits opportunities for improvement.

polyethene
  • Members
  • 5519 posts
  • Last active: May 17 2015 06:39 AM
  • Joined: 26 Oct 2012

I would have expected */ to generate a syntax error if there is no matching /*. It should not be ignored, it should generate a syntax error.

I disagree. By definition anything in code that is a comment should be ignored. It also serves a practical purpose - you could comment out the /* line itself if you need to quickly enable a section of code without having to look for the closing */.

The sequence */ should be treated as a single token, not as two operators. If the parser only feels like looking ahead one character then that's simply a reason for the bug, not a justification for a design.

Currently IronAHK also scans the first non-whitespace character for a continuation section with the exception of */. Perhaps for added clarity the documentation could be reworded to "the same is true for all other expression operator symbols".

And ; within a string should not begin a comment. It should be treated as a literal semicolon. I'd call that a bug too.

Yes, while it contradicts with my earlier statement that comments should take precedence IronAHK defers comment scanning to allow ; in string literals for expressions. Every other language is the same, AutoHotkey should be no different.

autohotkey.com/net Site Manager

 

Contact me by email (polyethene at autohotkey.net) or message tidbit


Lexikos
  • Administrators
  • 9844 posts
  • AutoHotkey Foundation
  • Last active:
  • Joined: 17 Oct 2006
Sorry, I hadn't properly explained my motivation:

Continuation lines are processed at an early stage, along with continuation sections and comments. This stage builds a line of code from one or more physical lines, preparing it for the later stages which handle hotkeys, function declarations, commands, expressions, etc. It seems to me that the implementation can either be complicated, duplicating much of the work done by the later stages in order to give the most consistent and sensible results; or (as it is now) simple and fast, giving adequate results in the majority of cases.

At the point continuation lines are processed, no "tokens" exist, only strings of characters. The current parser wasn’t designed from the start with expressions in mind, as they were introduced later. Redesigning it to tokenize everything at an early stage (or the only stage) would give the best results and would give benefits in other areas, but hardly seems justified.

"Fixing" only */ is simple:
if ((*next_buf == ':' || *next_buf == '+' || *next_buf == '-') && next_buf[1] == *next_buf 
	|| (*next_buf == '.' || *next_buf == '?') && !IS_SPACE_OR_TAB_OR_NBSP(next_buf[1])
		&& next_buf[1] != '=' 
[color=red]	|| (*next_buf == '*' && next_buf[1] == '/')[/color]
	|| !strchr(CONTINUATION_LINE_SYMBOLS, *next_buf)) [color=green]// Line doesn't start with a continuation char.[/color]
	break; [color=green]// Leave is_continuation_line set to its default of false.[/color]

And ; within a string should not begin a comment. It should be treated as a literal semicolon.

The documentation only states that the comment flag must have a space or tab to its left - there's nothing excluding it as a comment flag in a literal string. If you were to say "treating the comment flag as part of the string would be more useful and intuitive", I'd agree; but I disagree with your statement in principle (-> "should").
MsgBox % "Hello ; This line begins the string
        , World!" ; and this line ends it.
MsgBox Hello ; This line begins the string
     , World! ; and this line ends it.

... just like they do in every other language.

Every other language is the same, AutoHotkey should be no different.

I tend to ignore such suggestions when they aren't backed up with any actual reasoning, like why it is that way in other languages. If the reason is obvious (as for "foo ; bar"), there is no need to mention other languages and no added value in doing so. Similarity to any or all other languages is not a goal of AutoHotkey.

I understand that sometimes similarity to other languages can be a benefit, but the similarity itself isn't a reason - the users' familiarity with it is.

I believe that to defend existing behavior as "correct" through mental contortions does a disservice to AHK, because it inhibits opportunities for improvement.

Wasting time developing solutions for inconsequential "problems" inhibits opportunities for improvement where it actually matters.

[Ignoring unpaired */] also serves a practical purpose

I agree.

Perhaps for added clarity the documentation could be reworded to "the same is true for all other expression operator symbols".

"Symbol" isn't perfectly accurate, unfortunately.
MsgBox This is part of the message...
       and so is this.

Edit: Just because I like wasting time ;), making the following change to script.cpp is sufficient for causing */ at the beginning of a line to be entirely ignored if there wasn't a /*.
[color=red]if (in_comment_section)
{[/color]
    if (!_tcsncmp(next_buf, _T("*/"), 2))
    {
        [color=darkgray]...[/color]
    }
    else [color=green]if (in_comment_section)[/color]
        continue;
[color=red]}
else[/color] if (!in_continuation_section && !_tcsncmp(next_buf, _T("/*"), 2))
{
    in_comment_section = true;
    continue;
}
Does IronAhk ignore unpaired */?

Jamie
  • Members
  • 129 posts
  • Last active: Dec 02 2012 04:59 AM
  • Joined: 26 Mar 2010
Just because something is a bug does not mean that it has to be fixed. I'm fine with there being bugs that are decided to be not worth fixing.

But to me there is quite a difference between 'not worth fixing' and 'by design', even if they lead to the same outcome.

Maybe one day I will finally write that lint tool I keep talking about. Then people like me can stop complaining about things that should be illegal, but aren't. And by "should" I mean a personal judgment call, not a universal principle.

polyethene
  • Members
  • 5519 posts
  • Last active: May 17 2015 06:39 AM
  • Joined: 26 Oct 2012

At the point continuation lines are processed, no "tokens" exist, only strings of characters. The current parser wasn’t designed from the start with expressions in mind, as they were introduced later. Redesigning it to tokenize everything at an early stage (or the only stage) would give the best results and would give benefits in other areas, but hardly seems justified.

You've highlighted the fundamental problem. Many of the current design flaws and inconsistencies exist because of initial poor planning (since AutoIt 2) and failure to recognise how other programming languages were constructed to have a solid foundation for the syntax parser. Tobias (IronAHK co-developer) joked that it would be an impossibility to write ANTLR or EBNF grammar for AutoHotkey syntax, I didn't tell him he wasn't wrong. There are dozens of legacy syntax features gated by kill switches in IronAHK and I would have liked to discuss every one in a topic like this. Sadly though it seems that nobody is willing to take the responsibility for fixing AutoHotkey at its core and I have yet to see developers working in unison to move things forward. In the end we will end up with more confused users and wildly different forks/versions that have little common ground.

"Symbol" isn't perfectly accurate, unfortunately.

It was part of the longer sentence: Method #1: A line that starts with "and", "or", ||, &&, a comma, or a period is automatically merged with the line directly above it (in v1.0.46+, the same is true for all other expression operator symbols except ++ and --).

Does IronAhk ignore unpaired */?

Yes, note the commit referenced in my first post. Like ++ and -- it is not recognised as a continuation section and will be silently ignored if unpaired.

autohotkey.com/net Site Manager

 

Contact me by email (polyethene at autohotkey.net) or message tidbit