FWD » Core Development » Conversion Tools

When incremental conversion is ran, once the parse phase finishes, a file is marked 'converted'. But, incremental conversion may fail further in the conversion process - FWD should rollback the state of the files as 'unconverted', so they can be picked up again in the next incremental conversion run.

If incremental conversion fails during the Scanning Progress Source (preprocessor, lexer, parser, persist ASTs) phase, it will not be marked as "converted" and will be picked up when running incremental conversion again.

For the rest of the incremental conversion process, I though of letting the PatternEngine.run() throw and catch errors for each individual processAst() call, add those files to a set and have them eventually removed from the sources for the following phases. I got the first part done and it looks promising, but I need to let the PatternEngine know if I am running incremental or not.

• Considering #7318/#7179, should we remove files from conversion even if we are not running incremental conversion? If there are sources that failed to be converted, incremental conversion can be run after that.
• What should happen if global init/post rules fail in PatternEngine.run() and not processAst? This is for #1772, there is no specific file for this that can be used as reference for a possible conversion error manager.

Committed a null check in 6889a/16268.

Code Review Task Branch 6889a Revisions 16267 and 16268

1. Shouldn't cleanSources() be called at the end of processTrees() instead of in some of the callers? It seems like it should be called every time and it can be done in just 2 places instead of exploded out everywhere. Also, as part of this, I don't see why we need the failedSources set to be a data member. It can just be local data in processTrees(). That would simplify the changes.

2. The catch (Throwable thr) appears 3 times in the 2 PatternEngine.run() methods. That code seems pretty duplicative. Can you condense that down into a common worker method?

3. Didn't we see a regression recently with the code changed in FileList.removeFiles()? Perhaps I am misremembering.

Greg Shah wrote:

Code Review Task Branch 6889a Revisions 16267 and 16268

1. Shouldn't cleanSources() be called at the end of processTrees() instead of in some of the callers? It seems like it should be called every time and it can be done in just 2 places instead of exploded out everywhere. Also, as part of this, I don't see why we need the failedSources set to be a data member. It can just be local data in processTrees(). That would simplify the changes.

processTrees() is static, so I would be required to pass the TransformDriver instance as a parameter to this call or pass the original sources. There's also the option of creating another method to call it (I actually think this is better).

2. The catch (Throwable thr) appears 3 times in the 2 PatternEngine.run() methods. That code seems pretty duplicative. Can you condense that down into a common worker method?

I will check and create a separate method.

3. Didn't we see a regression recently with the code changed in FileList.removeFiles()? Perhaps I am misremembering.

I searched for FileList.removeFiles() in other issues and did not find any mention of it.

Code Review Task Branch 6889a Revision 16269

Literals like ".ast", ".p2o", ".jast", ".dict" should be AstGenerator constants. Otherwise the changes are very nice.

Regression tests passed, I am rebasing to check for conflicts.

Rebased 6889a to latest trunk/16312, the branch is now at revision 16316.

6889a can be merged.

Trunk is currently frozen for a delivery.

Alex: Please schedule this in the merge queue when trunk thaws.