No error when forgetting ¨;¨ in particle forces in the kinematicCloudProperties file
I made the really stupid mistake of forgetting the semicolon at the end of a particle force in the kinematicCloudProperties file.
WenYuDragForce { alphac alpha.water }
Because the statement is not ended, the next particle forces are not read in, which lead to really weird results. However, there is no warning of error. In many other input dictionaries the solver crashes because the next line is not read in.
I get that the user is responsible in the end for the input. However, in many other cases we are helped by errors and warnings. Therefore this mistake slipped by me. Would there be an easy fix to check this and give an error? I think this would be a nice safety feature. However, I can imagine that it would be too much work to prevent this kind of mistakes.
Activity
assigned to @mark
Some interesting ideas that you raise.
Properly handling bad input is indeed a vexing task. With issue #510 (closed) (leading to change
ddfbc7ed9e210143bf58a868ade9d49f64d02336) we tried to trap some possible cases. With change 13f04876894ac8a2132c5661c59de09bbc4e4e4c I started something similar for command-line entries - I didn't like the idea that trailing input rubbish should go undetected.For your issue, the problem is how we handle the dictionary input entries that are improperly terminated, or perhaps have additional trailing junk. In most cases we simply say something like this:
someType(dict.lookup("type"));The dictionary
lookup()returns a list of entry tokens represented as anITstream(an input token stream) that allows someType to read the word, leaving the stream positioned for the next possible read. This also means that things like the following work:div(phi,U) bounded Gauss linearUpwindV grad(U);To catch trailing rubbish, we could have something like
lookupOne()which would assert that the token stream has exactly one token and make loads of changes in the code, but this concept would start to fall apart if we are processing something slightly more complex. Eg,fields (U T p);where a single token assert isn't going to work. So I think we can forget this idea.
For your issue, we have the '}' being add into the previous token stream. So that you essentially have
alphac alpha.water }plus pretty much the entire balance of your dictionary as being the entry.
Having a '}' in the entry is perfectly OK, since we could have sub dictionaries etc. Maybe a reasonable ad hoc solution might be to trap unbalanced pairs of
{and}?Can we think of cases where this would a cause a false positive?
Thank you for taking an interest in this case. I think many compilers check for a balance in
{and}and I cannot think of a case where you would not want a closing bracket. I think this would be good solution.Another idea is to check if there is a semicolon between an entry the closing
}If there no semicolon there, you also know the location of the missing semicolon and could give an exact error message. Could this leads to weird behaviour?By Bas Nieuwboer on 2018-03-12T09:22:30 (imported from GitLab project)
-
The extra checks for mismatching the shapes of brackets (eg, `{)' mismatch) seems nice enough, but probably have to tackle it with two depth counters instead. I don't think it will manage the following at all:
some ( ( test ) }since the first closing
)resets the startBlock.Edited by Mark OLESEN mentioned in issue #843 (closed)
How about enforcing reading all optional entries to eof (i.e. enforcing the token stream to be one entry only)
See attached test app. Run on cavity case inside (with error in system/controlDict).
-
Taken yet another look, and this actually looks like a way to move forward - albeit still a long way. Your idea is currently just for optional entries (and lookupType, which only occurs once in the code).
If we have a
get<T>method with the same idea aslookupType<T>- transforming an entry to a single value and checking for excess tokens, we easily replace these types of constructions:var1_(readScalar(dict.lookup("key1"))) var2_(readLabel(dict.lookup("key2")))with
var1_(dict.get<scalar>("key1")) var2_(dict.get<label>("key2"))Which probably means that these will be the rule.
Edited by Mark OLESEN Master only dictionary reading does not transfer line numbers and file name.
E.g. master processor (upon error) correctly prints
0] entry 'order' has 2 excess tokens, near line: 25 dictionary: "/hosts/punwor073/home/pss/projects/GIT/dictionaryInputChecks/system/decomposeParDict.optional"slave processors print
entry 'order' has 2 excess tokens, near line: 0 dictionary: "IOstream"@mark: Any particular reason why they are not detected here? (as not optional entries?)
Is it that the bracket count should be zero at end of dictionary? Or only at end of IOdictionary?
-
Just looking into this - bracket counting was only for primitive entries...
Edited by Mark OLESEN -
The issue reported related to the handling of a missing brace or an extra brace. The problem arises from how
entry::Newis being used to parse dictionary entries. We need to have a check similar to the Istream::readEndList treatment - to manage expected values. mentioned in issue #1033 (closed)
