[PATCH] D24508: PR28752: Do not instantiate var decls which are not visible.

[Vassil Vassilev via cfe-commits]

v.g.vassilev added inline comments.


> rsmith wrote in SemaTemplate.cpp:509
> This function still appears to be able to return true (indicating to the caller that a diagnostic was produced) without actually producing a diagnostic.

Is it better now?

> rsmith wrote in SemaTemplate.cpp:505
> Why do we not issue a diagnostic in this case for a `VarDecl` when `Complain` is true and no definition is available? It seems like we should either be diagnosing this or asserting that it can't happen.

I believe it is not implemented, i.e. we didn't have this diagnostics when `VarDecl::getInstantiatedFromStaticDataMember` is true. I added a FIXME: for a future enhancement.

> rsmith wrote in ASTWriterDecl.cpp:896-897
> Sink this flag into the "not for `ParmVarDecl`" block below.

I thought we might need this for c-style `void f(struct S arg)`-like constructs where we might need to demote if we merge ParmVarDecls.

> rsmith wrote in ASTWriterDecl.cpp:1965
> Hmm. The width of the `InitStyle` field is definitely wrong right now, but should be fixed separately from this change. It looks like we don't hit this today because we don't use this abbreviation for a variable with an initializer. In addition to fixing the width of this field, we should also remove the `getInit() == nullptr` check when selecting the abbreviation in `ASTDeclWriter::VisitVarDecl`.

Committed in r283444.

https://reviews.llvm.org/D24508