Referencing not already defined targets in Alias() about fastbuild HOT 5 CLOSED

Targets must always be defined before use. If you reference something you haven't defined before, FASTBuild assumes you are referring to an external file and auto-creates a target for you. This is not Alias specific.

In the Alias example, as the dll and import lib are external targets, FASTBuild has to auto-create FileNodes for them.

The error could be better. Ideally we'd improve this by specifying where the original conflicting definition came from. This would require some non-trivial changes to bff parsing - we'd have to track a BFFIterator for every declared target, and cope with scoping problems when parsing things like includes (i.e. BFFIterator referencing a file that is no longer open)

from fastbuild.

In the Alias example, as the dll and import lib are external targets, FASTBuild has to auto-create FileNodes for them. This is not Alias specific.

I found these 2 snippets, which seems to correspond to what you described :

• FBuild\FBuildCore\BFF\Functions\Function.cpp(567)

    Node * n = ng.FindNode( nodeName );
    if ( n == nullptr )
    {
        // not found - create a new file node
        n = ng.CreateFileNode( nodeName );
        nodes.Append( Dependency( n ) );
        return true;
    }

• FBuild\FBuildCore\BFF\Functions\FunctionExecutable.cpp(229)

    // node not found - create a new FileNode, assuming we are
    // linking against an externally built library
    node = ng.CreateFileNode( nodeName );
    nodes.Append( Dependency( node ) );
    return true;

What would you think of adding a forward declaration syntax, like Library( 'LibraryAlias' ) ?
It requires to have a new Undefined state in Nodes to avoid access to properties of a forward declared node and to change a bit Function parser, but still might be a good improvement.

from fastbuild.

I took a look through the code, and there are at least 14 places which create FileNodes like this - essentially every Node type handles this case like this.

I think a forward declaration syntax would complicate the parsing a lot, as well as having consequences for things like error checking. The entire core of the BFF parsing has been built upon the assumption that you define something before you use it. I tend to think it's also reasonable that you can't use things you haven't defined yet - this being something programmers generally have to do.

I think the core problem is that the error is not helpful when you make a mistake and have a conflict. I think if the error was:

fbuild.bff(100,1): FASTBuild Error #1100 - Exec() - Target 'mylib' already defined.
Library( 'mylib' )
^
\--here
fbuild.bff(5,1): FASTBuild Error #1100 - Exec() - See previous declaration:
Exec( 'myexec' )
^
\--here

it would be much clearer.

This would help the case where you did define the same thing twice, either by including something twice (without #once) or by making a mistake in the declaration.

Implementing this isn't trivial either, but I think it'll be a lot easier/safer than changing the fundamental assumptions made in the parsing.

from fastbuild.

I made a change to Error #1100 such that it emits enhanced errors including the location of the initial declaration when a conflicting definition is encountered (f749794).

This change will be in the next release (v1.11). I'll leave this issue open until that version is released.

from fastbuild.

FASTBuild v1.11 has now been released and includes improved output for Error #1100 (information about the previous declaration will be emitted when that error occurs).

from fastbuild.