The Persistence of Vision Raytracer (POV-Ray).
This is the legacy Bug Tracking System for the POV-Ray project. Bugs listed here are being migrated to our github issue tracker. Please refer to that for new reports or updates to existing ones on this system.
Opened by Andrey Zholos - 2011-05-28
Last edited by William F Pokorny - 2016-11-15
FS#207 - Attempted to redefine float identifier as function identifier.
#macro A() #local f = function { x } #end #local f = 1; A()
This gives:
File 'bug.pov' line 2: Parse Error: Attempted to redefine float identifier as function identifier.
The problem is that this makes using functions in library macros difficult. Basically, they must have a globally unique name that’s not used in any of the macros or files that call the macros. #undef doesn’t really help, because it destroys the identifier in the calling scope.
For example, one of the macros in the standard include files names a function “fn”, so this doesn’t work:
#include "transforms.inc" #local fn = 42; // fnord? #local fn_pos = vtransform(x, transform { rotate 30*y } );
The reason for this restriction is explained in Parse_RValue in source/backend/parser/parse.cpp:
// Do NOT allow to redefine functions! [trf] // #declare foo = function(x) { x } // #declare foo = function(x) { foo(x) } // Error! // Reason: Code like this would be unreadable but possible. Is it // a recursive function or not? - It is not recursive because the // foo in the second line refers to the first function, which is // not logical. Further, recursion is not supported in POV-Ray 3.5 // anyway. However, allowing such code now would cause problems // implementing recursive functions after POV-Ray 3.5!
In this case the restriction is applied too broadly: it should be safe to redefine anything other than a function to a function and still avoid it looking like recursion. In fact, there’s a restriction in Parse_Declare specifically to prevent redefining functions.
Tuesday, 15 November 2016, 16:08 GMT
Reason for closing: Fixed
Additional comments about closing:
Fixed in current 3.7.1 master branch by commit 7d06e7f was Christoph Lipka, Fri Sep 9 09:50:47 2016 +0200, message: Fixed some parser flaws related to the re-defining of variables.
Thanks for not only finding & reporting this issue, but even digging into the code.
While it might be debated whether this is strictly an error, it does go against the idea of local variables severely enough to be classified as one either way.
Note that the suggested solution would only partially solve the issue: If a global function already exists with that name (and this might be pretty common for names such as “fn”), an error would still be reported.
I think the proper solution would be to check only the local variables whether the function name is already in use. A warning might still be printed if the name exists as a global variable. (Note that “local” variables at main scene file scope are actually global.)
There's also a similar problem with macro names and local variables of any kind.
As for redefining functions, I would vote for removing the check altogether and treating functions as just another kind of value, like lambda expressions in other languages. Self-recursion like in the comment can be implemented with a constant that refers to the current function, e.g.
Right now declarations of functions behave as if they are named functions, and I think the syntax for that should be separate from declarations of variables, if the feature is required at all.
For example, how should mutual recursion look?
The purpose of preventing recursion (or rather detect it) is that it is a not fully tested and not officially supported.
Apart from this restriction, it is generally not the best idea to use functions as local macro variables, as very odd things can be constructed using local macro variables and functions.
Of course, one day these restrictions should be lifted, but not in 3.7 as the possible side-effects of breaking something elsewhere are very difficult to predict due to the age of the POV-Ray parser code.