On Thu, Jul 18, 2013 at 5:51 PM, Eli Friedman <[email protected]>wrote:
> New patch attached. It should be much more clear what it is and is > not doing. (Also added the template template parameter test.) > LGTM (but we seem to have some remaining problems -- see below) > On Wed, Jul 17, 2013 at 6:58 PM, Eli Friedman <[email protected]> > wrote: > > On Wed, Jul 17, 2013 at 6:01 PM, Richard Smith <[email protected]> > wrote: > >> On Wed, Jul 17, 2013 at 5:43 PM, Eli Friedman <[email protected]> > >> wrote: > >>> > >>> Patch attached. Fixes PR16646. The concept is that when substitution > >>> replaces a pack with another pack, we don't want to expand the pack > >>> immediately: we want to expand it at the same level as the original > >>> pack. > >> > >> > >> How do we ensure that the we retain a pack expansion around the > transformed > >> entity? > > > > I'm pretty sure that if we find a template argument which is a pack > > expansion, the input TemplateTypeParmType has to be an unexpanded > > parameter pack; if I'm wrong about that, the code should be moved a > > bit. I'll probably end up moving it anyway to make it more clear. > > > > If we are expanding a parameter pack, we must be in the context of an > > ArgumentPackSubstitutionIndexRAII which should expect the possibility > > of an unexpanded parameter pack. > Some (but not all) cases where we expand parameter packs do detect the case where the transformed result contains an unexpanded pack and rebuild a pack expansion around it. Looking for callers of TryExpandParameterPacks in TreeTransform, I see: TransformExprs, TransformTemplateArguments, TransformObjCDictionaryLiteral get this right. TransformFunctionTypeParams, TransformTypeTraitExpr, TransformLambdaScope get this wrong. TransformSizeOfPackExpr gets this very wrong -- that's PR14858. >>> I'm not confident that this patch is correct, though: just stripping > >>> off the PackExpansionType in the middle of instantiation seems weird. > >>> Also, I'm not sure what other tests would be relevant. > >> > >> > >> The patch only covers type template parameters; presumably, the same > issue > >> applies to non-type template parameters and template template > parameters? > > > > We have code which I'm pretty sure is equivalent for non-type template > > parameters; see the beginning of > > TemplateInstantiator::transformNonTypeTemplateParmRef. The following > > testcase works without any other modifications: > > > > template <int x> struct DefaultValue { const int value = x;}; > > template <typename ... Args> struct tuple {}; > > template <int ... Args> using Zero = tuple<DefaultValue<Args> ...>; > > template <int ... Args> void f(const Zero<Args ...> &t); > > void f() { > > f(Zero<1,2,3>()); > > } > > > > On the other hand, the following currently crashes: > > > > template<int x> struct X {}; > > template <template<int x> class temp> struct DefaultValue { const > > temp<0> value; }; > > template <typename ... Args> struct tuple {}; > > template <template<int x> class... Args> using Zero = > > tuple<DefaultValue<Args> ...>; > > template <template<int x> class... Args> void f(const Zero<Args ...> &t); > > void f() { > > f(Zero<X,X,X>()); > > } > > > > Conceptually, it's the same sort of fix, though. > > > > I should probably use TemplateArgument::getPackExpansionPattern > > consistently, instead of directly checking for a PackExpansionType. > > > >> Does this correctly handle expanding the pack into a fixed set of > parameters > >> and a pack, in cases like: > >> > >> template<typename ...Ts> struct S {}; > >> template<typename ...Ts> using X = S<S<Ts>...>; > >> template<typename ...Ts> void f(X<int, Ts...> x); > >> void g() { f(X<int, int, int>()); } > >> > >> ? (I think it should, but this seems worth testing.) > > > > It does. > > > > Thanks for asking the right questions. I learned a lot more about my > > patch by writing up the answers. :) >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
