Using non-qualified identifiers as DUContext scope IDs

[Nicolás Alvarez]

Hi,

This is a continuation of my previous email about openDeclaration.

AbstractContextBuilder::openContext also takes a QualifiedIdentifier,
where it seems like taking a single Identifier would make more sense.
However, while a Declaration stores an Identifier (and openDeclaration
uses the last Identifier from a QID), a DUContext actually stores a
QualifiedIdentifier as its "localScopeIdentifier", so it's a tricker
situation. We need to look at that property too.

I think localScopeIdentifier should be an Identifier too. The full
scopeIdentifier() of a DUContext is formed by walking the parent
contexts and concatenating their Identifiers, so I don't see why each
local scope ID should be a QID. I think if you have multiple
components in a QID, you should have multiple nested contexts too.

The problem is that kdev-clang often creates DUContexts with
multi-component QIDs (ns::class or ns1::ns2::class), as I verified by
adding an assertion. Interestingly, many of these came from
kdev-clang's Visitor::createContext, which has this lovely comment:

"// wtf: why is the DUContext API requesting a QID when it needs a plain Id?!"

Making things even more interesting, I looked at a few cases where
kdev-clang was creating DUContexts with multi-component QIDs as scope
identifiers, and I found they are *wrong*. Here is an example
testcase:

namespace ns{} // necessary, don't know why

namespace ns {
    class vec {
    public:
        void reserve(int n);
    };
    void vec::reserve(int n) {
        int x=12;
        x+=2; //avoid "unused variable" warning
    }
}

At some point this will create the internal context for the reserve()
definition (at least that's what I think it is), with
localScopeIdentifier="ns::vec" (ie. a QualifiedIdentifier with
[Identifier("ns"), Identifier("vec")]). Its parent context is the
'foo' namespace. The full scopeIdentifier() of the context is thus
"ns::ns::vec", which is clearly wrong. This is even visible in the
GUI! The tooltip of the 'x' variable says "Scope:
ns::ns::vec::reserve".

This is clearly a bug, but I think it's not *just* a concrete bug in
kdev-clang that we fix and move on. I think there is *no* case where
it would make sense for a DUContext to have multiple ID components as
its scope QID. You should just create multiple nested contexts and
assign each a single identifier.

I propose:
1. add a warning or assertion in DUContext to ensure id.count() <= 1
2. fix problems in kdev-clang that trigger the above warning or
assertion (and maybe in other language plugins; I didn't try)
3. now that everyone passes a single-component QID, change DUContext
to store an Identifier instead of a QualifiedIdentifier, and change
APIs that create contexts accordingly

Comments?

-- 
Nicolás