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.

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
ddfbc7ed) we tried to trap some possible cases. With change 13f04876 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 an ITstream (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?

hacked this together

primitiveEntryIO.C

seems to run cavity tutorial ok ;-)

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.

mentioned in commit b1d2f466

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).

dictionary_with_warning.tgz

This obviously won't cover everything, but is a good start. I'll add it in.

Please check since it should only be applied to optional entries. I have not run anything else but my little test.

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 as lookupType<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.

mentioned in commit e48dd3c1

mentioned in commit 84fcc732

mentioned in commit 2549500d

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"

mentioned in commit bb2b90ef

@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...

The issue reported related to the handling of a missing brace or an extra brace. The problem arises from how entry::New is being used to parse dictionary entries. We need to have a check similar to the Istream::readEndList treatment - to manage expected values.

mentioned in commit 79fee8f7b58e8c4d590ffdc944f93fe5d8b76ab2

@Prashant @svilfaye - Made changes and pushed onto branch issue-762. I think that these changes will address more of these types of brace issues, but since it has the risk of introducing a regression it will not be added in until after the 1806 release.

mentioned in commit 8e317427

mentioned in commit 77420591

mentioned in commit 1ca45420

mentioned in commit af7fd85f

mentioned in commit f057060d

mentioned in commit 14393672

mentioned in commit d8c49fc8

mentioned in commit 532a3d3c

mentioned in commit f2049c58

mentioned in issue #1033 (closed)

mentioned in commit 392ef54c

mentioned in commit 45c8503c

mentioned in commit efe02691

mentioned in commit e80be584

mentioned in commit fdb5ae09

mentioned in commit 1cc0e66f

mentioned in commit 9a5038a8

closed