[PATCH] D10834: Added functions to retrieve information about variable storage in libclang and its python bindings.

[Argyrios Kyrtzidis via cfe-commits]

> On Oct 15, 2015, at 2:06 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> rsmith added a comment.
> 
> Argyrios, I'd appreciate your thoughts here.
> 
> 
> ================
> Comment at: tools/libclang/CIndex.cpp:6670-6694
> @@ -6669,1 +6669,27 @@
> 
> +bool clang_Cursor_hasLocalStorage(CXCursor C) {
> +  if (C.kind != CXCursor_VarDecl) {
> +    return false;
> +  }
> +
> +  const Decl *D = getCursorDecl(C);
> +  if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
> +    return VD->hasLocalStorage();
> +  }
> +
> +  return false;
> +}
> +
> +bool clang_Cursor_isStaticLocal(CXCursor C) {
> +  if (C.kind != CXCursor_VarDecl) {
> +    return false;
> +  }
> +
> +  const Decl *D = getCursorDecl(C);
> +  if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
> +    return VD->isStaticLocal();
> +  }
> +
> +  return false;
> +}
> +
> ----------------
> I don't think the names of these are really specific enough for what they do. As members of `VarDecl`, they're good enough, but as operations on a general `Cursor`, it's less so. For instance, temporary objects in C++ might also have local storage or be static locals. Renaming `isStaticLocal` to `isStaticLocalVar` would help.
> 
> `hasLocalStorage` is not really a very user-friendly name for this functionality, even though it currently matches our C++ API (the C API has long-term stability guarantees whereas the C++ API does not, so we need to take more care when naming C API functions, even though matching the C++ API does generally make the C API more user-friendly). `isStaticLocalVar` versus `isNonStaticLocalVar` might be better if you don't want to go the enum route.

I agree with Richard, an enum is better. To the enums Richard proposed I’d suggest adding invalid: { invalid, not local, non-static local, static local }.

> 
> 
> http://reviews.llvm.org/D10834
> 
> 
>