sleyzerzon / soar

Automatically exported from code.google.com/p/soar
1 stars 0 forks source link

Documentation on acceptable variable names is incorrect #113

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
In the Soar manual section 3.3.5.2, it states that a variable is a "symbol that 
begins with a left angle-bracket (i.e., <), ends with a right angle-bracket 
(i.e., >), and contains at least one alphanumeric symbol in between."
This isn't true, or at least Soar doesn't enforce it. The following sources 
just fine:

sp {dash
    (state <->)
--> (<-> ^foo <bar>)
}

The BNF grammar in the back (Appendix B) is also incorrect. It says that a 
variable consists of a left and right bracket, with a sym_constant inside. 
Assuming that sym_constant is defined as the symbolic constant mentioned in 
3.1.1, then it accepts the quoted vertical bar syntax (|some spaces here|, 
etc.). Variable names will not accept this:
sp {vertical-bar-test
    (state <|a b|>)
--> (<|a b|> ^foo <bar>)
}

I suggest that sym_constant have a rule in the BNF grammar, showing that it can 
be either [A-Za-z0-9\$%&*+\/:=?_-]+ or "|" [^|]+ "|". These sequences should be 
given names (since the grammar doesn't list any actual regular expressions like 
these), and the first entry should be used in the variable syntax:

<variable> ::= "<" <string> ">"
<string>   ::= any combo of letters, digits, or $%&*+-/:<=>?_
<quoted>   ::= "|" <any_string> "|"
<sym_constant> ::= <string> | <quoted>.

The description of of variable syntax should then be changed in 3.3.5.2 to be 
similar to that of the symbolic constant: any combination of letters, digits, 
or $%&*+-/:<=>?_ .

Original issue reported on code.google.com by garfield...@gmail.com on 24 Sep 2012 at 8:25

GoogleCodeExporter commented 8 years ago
Unfortunately, I'm sure there are many other inconsistencies between the 
grammar described in the manual and the parsing code, which was written by hand 
many years ago, not generated by the likes of yacc from a formal grammar. 
People (me included) have tried to write grammars for generating "correct" 
parsers in the past, but they always ended up breaking on obscure (but real) 
cases. I think the parsing code is just too permissive to describe in a 
succinct grammar, and we don't want to break existing Soar code by introducing 
a more restrictive parser.

My recommended fix is to include a warning blurb in the manual telling people 
not to rely on that grammar being 100% correct.

Original comment by joseph...@gmail.com on 24 Sep 2012 at 8:49

GoogleCodeExporter commented 8 years ago
While we're talking about the grammar, there's one more thing. The manual says 
a <condition-side> is <state-imp-cond> <cond>*, but it should be simply 
<cond>+. This is how it appears in the comments in Parser.cpp.

Original comment by garfield...@gmail.com on 6 Oct 2012 at 5:55

GoogleCodeExporter commented 8 years ago
Not sure if anyone still cares about this issue, but I have a formal grammar 
for Soar files (attached) that parses a large proportion of Soar code in this 
repository. The grammar is incomplete and overly verbose (and in some personal 
variable of EBNF), but it works. Some interested person can use this as a basis 
for creating a yacc-parsable grammar.

Original comment by justinn...@gmail.com on 8 Feb 2013 at 4:20

Attachments:

GoogleCodeExporter commented 8 years ago
Neat! Thanks. I made another one, trying to follow the behavior of the CSoar 
parser, here: https://metacpan.org/module/Soar::Production::Parser::PRDGrammar. 
Doesn't have any of the TCL stuff that yours does.

Original comment by garfield...@gmail.com on 10 Feb 2013 at 12:54

GoogleCodeExporter commented 8 years ago
I just read your bug section. I don't know about it being a "known" bug (I only 
encountered it about a week ago), but see issue 135.

Original comment by justinn...@gmail.com on 10 Feb 2013 at 4:33

GoogleCodeExporter commented 8 years ago
Since I was trying to mimic the Soar parser, the fact that it can parser 
"^hello.1.yes" while the actual Soar parser cannot made it a bug. I hadn't 
thought of reporting it as a bug in the Soar parser! Thought it was on purpose. 

Original comment by garfield...@gmail.com on 10 Feb 2013 at 8:19