PR17800: When performing pack expansion, we must always rebuild the AST nodes

to avoid breaking AST invariants by reusing Stmt nodes within the same
function.

llvm-svn: 194217

Author: zygoloid

clang/lib/Sema/TreeTransform.h

@@ -142,7 +142,11 @@ class TreeTransform {
   ///
   /// Subclasses may override this function to specify when the transformation
   /// should rebuild all AST nodes.
-  bool AlwaysRebuild() { return false; }
+  ///
+  /// We must always rebuild all AST nodes when performing variadic template
+  /// pack expansion, in order to avoid violating the AST invariant that each
+  /// statement node appears at most once in its containing declaration.
+  bool AlwaysRebuild() { return SemaRef.ArgumentPackSubstitutionIndex != -1; }
 
   /// \brief Returns the location of the entity being transformed, if that
   /// information was not available elsewhere in the AST.

clang/test/SemaCXX/constant-expression-cxx11.cpp

@@ -1811,3 +1811,16 @@ namespace NeverConstantTwoWays {
         1 / 0 : // expected-warning {{division by zero}} expected-note {{division by zero}}
         0;
 }
+
+namespace PR17800 {
+  struct A {
+    constexpr int operator()() const { return 0; }
+  };
+  template <typename ...T> constexpr int sink(T ...) {
+    return 0;
+  }
+  template <int ...N> constexpr int run() {
+    return sink(A()() + N ...);
+  }
+  constexpr int k = run<1, 2, 3>();
+}