Fold expressions miss parentheses

[Ukilele]

Hello Andreas, there might be an issue with the transformation of fold expressions.
Following code:

void f(int) { }

void TestA(auto... args) {
  f((... , args));
}

void TestB(auto... args) {
  int b = (... , args);
}

template<int... Is>
void TestC() {
  f((... , Is));
}

template<int... Is>
void TestD() {
  int d = (... , Is);
}

int main() {
  TestA(10, 15);
  TestB(7, 8);
  TestC<1, 2, 3>();
  TestD<4, 5, 6>();
} 

generates following outcome:

void f(int)
{
}


void TestA(auto... args) {
  f((... , args));
}

/* First instantiated from: insights.cpp:22 */
#ifdef INSIGHTS_USE_TEMPLATE
template<>
void TestA<int, int>(int __args0, int __args1)
{
  f(__args0 , __args1);
}
#endif


void TestB(auto... args) {
  int b = (... , args);
}

/* First instantiated from: insights.cpp:23 */
#ifdef INSIGHTS_USE_TEMPLATE
template<>
void TestB<int, int>(int __args0, int __args1)
{
  int b = __args0 , __args1;
}
#endif


template<int ...Is>
void TestC()
{
  f((... , Is));
}


/* First instantiated from: insights.cpp:24 */
#ifdef INSIGHTS_USE_TEMPLATE
template<>
void TestC<1, 2, 3>()
{
  f((1 , 2) , 3);
}
#endif


template<int ...Is>
void TestD()
{
  int d = (... , Is);
}


/* First instantiated from: insights.cpp:25 */
#ifdef INSIGHTS_USE_TEMPLATE
template<>
void TestD<4, 5, 6>()
{
  int d = (4 , 5) , 6;
}
#endif


int main()
{
  TestA(10, 15);
  TestB(7, 8);
  TestC<1, 2, 3>();
  TestD<4, 5, 6>();
  return 0;
}
 

In all four of my Test-functions, the issue is, that the fold expression does not contain the right amount of parentheses around them. According to https://eel.is/c++draft/temp.variadic#11, a unary left fold of the form (... , Is) should expand to ( ((Is1, Is2) , ...) , IsN ). But this is not, what cppinsights generates.
I added four Test-functions. TestA and TestB test the behavior with a function parameter pack, while TestC and TestD test the behavior with a template parameter pack. TestA and TestC use the fold expression within a function call, while TestC and TestD use the fold expression in an initialization. I did not check the behavior for the other kind of fold expressions (e.g. binary right fold) and other kind of operators, but I guess the behavior will be similar?

Cheers!

[Ukilele]

I quickly checked that at least for the operators , (comma), + (plus) and & (bit and) the behavior is the same.

[andreasfertig]

Hello @Ukilele,

thanks for reporting this issue!

I agree with your quote from the standard. My issue is that at the point I render something like 4, 5, 6, I don't know whether this results from a fold-expression or a user's code. Looking at the AST using Compiler Explorer compiler-explorer.com/z/MYWb9T3xq shows in line 77 the BinaryOperator. It does not say anything about a fold expression. I thought I could use NonTypeTemplateParmDecl in line 80, but this also doesn't know how the pack is used later. I would be happy if you or anybody else knows how to get this information.

C++ Insights introduces parens in some cases, for example

cppinsights/CodeGenerator.cpp

Line 720 in 50b962d

const bool needLHSParens{isa<BinaryOperator>(stmt->getLHS()->IgnoreImpCasts())};

I wonder how your test looked for + because my example works:

template<typename... Ts>
constexpr auto avg(const Ts&... vals)
{
  return (vals + ...) / sizeof...(vals);
}

static_assert(avg(2, 3, 4) == 3);

This translates to

template<>
inline constexpr unsigned long avg<int, int, int>(const int & __vals0, const int & __vals1, const int & __vals2)
{
  return (static_cast<unsigned long>(__vals0 + (__vals1 + __vals2))) / 3;
}

As much as it bugs me, this issue will remain unfixed for now.

Andreas

[Ukilele]

Hello @andreasfertig,

1.) You're welcome. Thanks so much for this helpful tool! 🚀

2.) Yes, it seems that once you have a concrete FunctionDecl (even if it is the result of instantiating a FunctionTemplateDecl),
all information about the contained fold expression is lost. Maybe via getDescribedFunctionTemplate you can get from the FunctionDecl to its FunctionTemplateDecl, which does contain the CXXFoldExpr we need. Still, it sounds like this issue is quite tedious to resolve.

3.) Thanks for pointing at const bool needLHSParens{isa<BinaryOperator>(stmt->getLHS()->IgnoreImpCasts())};, I only just now realized that cppinsights transforms 1 + 2 + 3 into (1 + 2) + 3.
I guess this explains the parentheses that cppinsights generates in the instantiations of my TestC and TestD functions.

4.) My test for + looked the way, that I took the original code I mentioned in the issue and replaced each occurrence of the fold-operator , by +.

Best regards,
Kilian

[andreasfertig]

Hello @Ukilele,

re 2.)

Thanks for that idea. Sadly, even with getDescribedFunctionTemplate, the transformation remains guesswork, and there is at least one scenario where it fails.

The results for TestA and TestC are different. In TestC, I can guess, with the help of storing state, that a SubstNonTypeTemplateParmExpr comes from a NonTypeTemplateParmDecl, which shares the same DeclRefExpr as used prior for defining the CXXFoldExpr in the primary template. I'm not even sure in how many cases this guessing fails, but at least for this case, it would work.

Everything is different for TestA. Here, we are looking at a ParmVarDecl in the instantiation. As far as I can tell, this decl shares nothing with the decl in the CXXFoldExpr.

With the guessing, the effort, and the situation that it would still not work in all cases, I do not plan to do anything here. However. as always, PRs are welcome.

Andreas