Thursday, 29 May 2014

The early-move kraken, incremental analysis and error handling.

I've gotten into the habit of compulsively moving things. It doesn't help that my current design involves a bunch of unique_ptr. So I've just fixed a bunch of bugs in code that looks like this:

OverloadSet* Analyzer::GetOverloadSet(std::unordered_set<clang::NamedDecl*> decls, ClangTU* from, Type* context) {
    if (clang_overload_sets.find(decls) == clang_overload_sets.end()
     || clang_overload_sets[decls].find(context) == clang_overload_sets[decls].end()) {
        clang_overload_sets[decls][context] = Wide::Memory::MakeUnique<OverloadSet>(std::move(decls), from, context, *this);
    }
    return clang_overload_sets[decls][context].get();
}


First, here we're depending on compiler initialization order to evaluate the left-hand-side first, because in the right-hand-side we destroy decls's contents. Even more hilariously, after that, we then use decls AGAIN to get the object we just inserted. Unsurprisingly, this piece of code did not work fantastically well. What's more surprising is how few tests failed.

    return GetSignature()->BuildCall(Wide::Memory::MakeUnique<Self>(this, !args.empty() ? args[0].get() : nullptr, std::move(val)), std::move(args), c);

 
Similar problem here. We've read args and moved from it in the same function call. Ignoring potential UB due to evaluation order, which I believe is safe because all operations are in terms of user-defined functions, the compiler can (and GCC did) move args first. There are likely many more such places in the Wide codebase (yay!).

Incremental analysis and error handling. So far I don't have any tests about incremental analysis or error recovery, we still use the exception error handling model. But incremental analysis is beginning to concern me because I don't have a solid model for which nodes are responsible for handling what. The core problem is that some nodes but certainly not all would have to be written to listen for changes to their input and adjust appropriately. I just don't know which ones and why.

No comments:

Post a Comment