[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-14 Thread Gauthier via Phabricator via cfe-commits
Tyker updated this revision to Diff 195056.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,32 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i, int b) {
+  i += 4;
+  b -= 4;
+  // expected-note@+1 {{negative shift count -7}}
+  return i << b;
+}
+
+constexpr int f2(int i) {
+  int b = 0;
+  i += 1;
+  b -= 1;
+  // expected-note@+1 {{in call to 'f1(3, -3)'}}
+  return f1(i + 2, b -= 2);
+}
+
+constexpr int f(int i) {
+  i -= 1;
+  //expected-note@+1 {{negative shift count -1}}
+  return 1 << i;
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{in call to 'f2(0)'}}
+constexpr int val = f2(0);
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{in call to 'f(0)'}}
+static_assert(f(0), "");
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -661,6 +661,10 @@
 /// CurrentCall - The top of the constexpr call stack.
 CallStackFrame *CurrentCall;
 
+/// ArgumentCallStack - Used to store Arguments to function calls
+/// only required if diagnostics should produce a callstack
+SmallVectorImpl *ArgumentCallStack;
+
 /// CallStackDepth - The number of calls in the call stack right now.
 unsigned CallStackDepth;
 
@@ -789,14 +793,14 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext , Expr::EvalStatus , EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  ArgumentCallStack(nullptr), CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue ) {
   EvaluatingDecl = Base;
@@ -1240,10 +1244,20 @@
   Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
+  if (Info.ArgumentCallStack) {
+Info.ArgumentCallStack->reserve(Info.ArgumentCallStack->size() +
+Callee->param_size());
+std::copy(Arguments, Arguments + Callee->param_size(),
+  std::back_inserter(*Info.ArgumentCallStack));
+  }
 }
 
 CallStackFrame::~CallStackFrame() {
   assert(Info.CurrentCall == this && "calls retired out of order");
+  if (Info.ArgumentCallStack && Caller) {
+Info.ArgumentCallStack->resize(Info.ArgumentCallStack->size() -
+   Callee->param_size());
+  }
   --Info.CallStackDepth;
   Info.CurrentCall = Caller;
 }
@@ -1257,9 +1271,12 @@
   return Result;
 }
 
-static void describeCall(CallStackFrame *Frame, raw_ostream );
+static void describeCall(CallStackFrame *Frame, raw_ostream ,
+ SmallVectorImpl *ArgumentCallStack,
+ unsigned Pos);
 
 void EvalInfo::addCallStack(unsigned Limit) {
+  assert(ArgumentCallStack && "needed to produce a call stack");
   // Determine which calls to skip, if any.
   unsigned ActiveCalls = CallStackDepth - 1;
   unsigned SkipStart = ActiveCalls, SkipEnd = SkipStart;
@@ -1270,8 +1287,10 @@
 
   // Walk the call stack and add the diagnostics.
   unsigned CallIdx = 0;
+  unsigned Pos = ArgumentCallStack->size();
   for (CallStackFrame *Frame = CurrentCall; Frame != 
Frame = Frame->Caller, ++CallIdx) {
+Pos -= Frame->Callee->param_size();
 // Skip this call?
 if (CallIdx >= SkipStart && CallIdx < SkipEnd) {
   if (CallIdx == SkipStart) {
@@ -1294,7 +1313,7 @@
 
 SmallVector Buffer;
 llvm::raw_svector_ostream Out(Buffer);
-describeCall(Frame, Out);
+describeCall(Frame, Out, ArgumentCallStack, Pos);
   

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-14 Thread Gauthier via Phabricator via cfe-commits
Tyker updated this revision to Diff 195055.
Tyker added a comment.

i changed the way arguments are stored. to make it more controllable.
added an argument call stack where it is needed.
this version slows down compilation by around 0.5% in average over 200 run for 
SemaDecl -fsyntax-only


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,32 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i, int b) {
+  i += 4;
+  b -= 4;
+  // expected-note@+1 {{negative shift count -7}}
+  return i << b;
+}
+
+constexpr int f2(int i) {
+  int b = 0;
+  i += 1;
+  b -= 1;
+  // expected-note@+1 {{in call to 'f1(3, -3)'}}
+  return f1(i + 2, b -= 2);
+}
+
+constexpr int f(int i) {
+  i -= 1;
+  //expected-note@+1 {{negative shift count -1}}
+  return 1 << i;
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{in call to 'f2(0)'}}
+constexpr int val = f2(0);
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{in call to 'f(0)'}}
+static_assert(f(0), "");
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -661,6 +661,10 @@
 /// CurrentCall - The top of the constexpr call stack.
 CallStackFrame *CurrentCall;
 
+/// ArgumentCallStack - Used to store Arguments to function calls
+/// only required if diagnostics should produce a callstack
+SmallVectorImpl *ArgumentCallStack;
+
 /// CallStackDepth - The number of calls in the call stack right now.
 unsigned CallStackDepth;
 
@@ -789,14 +793,14 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext , Expr::EvalStatus , EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  ArgumentCallStack(nullptr), CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue ) {
   EvaluatingDecl = Base;
@@ -1240,10 +1244,20 @@
   Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
+  if (Info.ArgumentCallStack) {
+Info.ArgumentCallStack->reserve(Info.ArgumentCallStack->size() +
+Callee->param_size());
+std::copy(Arguments, Arguments + Callee->param_size(),
+  std::back_inserter(*Info.ArgumentCallStack));
+  }
 }
 
 CallStackFrame::~CallStackFrame() {
   assert(Info.CurrentCall == this && "calls retired out of order");
+  if (Info.ArgumentCallStack && Caller) {
+Info.ArgumentCallStack->resize(Info.ArgumentCallStack->size() -
+   Callee->param_size());
+  }
   --Info.CallStackDepth;
   Info.CurrentCall = Caller;
 }
@@ -1257,9 +1271,12 @@
   return Result;
 }
 
-static void describeCall(CallStackFrame *Frame, raw_ostream );
+static void describeCall(CallStackFrame *Frame, raw_ostream ,
+ SmallVectorImpl *ArgumentCallStack,
+ unsigned );
 
 void EvalInfo::addCallStack(unsigned Limit) {
+  assert(ArgumentCallStack && "needed to produce a call stack");
   // Determine which calls to skip, if any.
   unsigned ActiveCalls = CallStackDepth - 1;
   unsigned SkipStart = ActiveCalls, SkipEnd = SkipStart;
@@ -1270,8 +1287,10 @@
 
   // Walk the call stack and add the diagnostics.
   unsigned CallIdx = 0;
+  unsigned Pos = ArgumentCallStack->size();
   for (CallStackFrame *Frame = CurrentCall; Frame != 
Frame = Frame->Caller, ++CallIdx) {
+Pos -= Frame->Callee->param_size();
 // Skip this call?
 if (CallIdx >= 

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-13 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment.

In D60561#1465373 , @Tyker wrote:

> the impact was much higher than i expected,  around 1% slower in average on 
> 50 compilation of SemaDecl with -fsyntax-only.


Good to know, thanks for doing the measurement !


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-13 Thread Gauthier via Phabricator via cfe-commits
Tyker added a comment.

the impact was much higher than i expected,  around 1% slower in average on 50 
compilation of SemaDecl with -fsyntax-only.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-12 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment.

In D60561#1464740 , @Tyker wrote:

> @rsmith i don't think collecting theses values is expansive compared to 
> evaluating the expression.
>  but i agree that we can disable the collection of these values when no 
> diagnostics can be emited.


To help make an informed decision you can grab a big TU (I usually use 
`SemaDecl.cpp` or all of Boost) and then run `-fsyntax-only` on it. With a tool 
like `perf stat` you can get pretty good results.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-12 Thread Gauthier via Phabricator via cfe-commits
Tyker added a comment.

@rsmith i don't think collecting theses values is expansive compared to 
evaluating the expression.
but i agree that we can disable the collection of these values when it isn't 
needed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-11 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

This seems liable to be moderately expensive, especially for large argument 
values, and it's a cost that is unnecessary in the common case where evaluation 
succeeds.

I wonder if we'd be better off speculatively trying the entire evaluation 
without storing any such values, and rerunning the evaluation to collect 
diagnostics only when we find that evaluation would fail (and we're in a 
context where the caller actually wants the diagnostics).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-11 Thread Gauthier via Phabricator via cfe-commits
Tyker updated this revision to Diff 194684.
Tyker added a comment.

@Quuxplusone  edited based of feedback

i simplified the test but didn't put the original because this one test 
arguments for variable and literal and there may be corner case differences


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,18 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i) {
+  i -= 1;
+  return 1 << i; // expected-note {{negative shift count -2}}
+}
+
+constexpr int f2(int i) {
+  i -= 1;
+  // expected-note@+1 {{'f1(-1)}}
+  return f1(i);
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{'f2(0)}}
+constexpr int a = f2(0);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -476,9 +476,14 @@
 const LValue *This;
 
 /// Arguments - Parameter bindings for this function call, indexed by
-/// parameters' function scope indices.
+/// parameters' function scope indices. may be modified by called function
 APValue *Arguments;
 
+/// ConstantArgs - Parameter bindings for this function call, indexed by
+/// parameters' function scope indices. can't be modified. used for
+/// diagnostics
+const APValue *ConstantArgs;
+
 // Note that we intentionally use std::map here so that references to
 // values are stable.
 typedef std::pair MapKeyTy;
@@ -519,7 +524,7 @@
 
 CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments);
+   APValue *Arguments, const APValue *ConstantArgs);
 ~CallStackFrame();
 
 // Return the temporary for Key whose version number is Version.
@@ -789,14 +794,15 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext , Expr::EvalStatus , EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr,
+  nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue ) {
   EvaluatingDecl = Base;
@@ -1235,9 +1241,10 @@
 
 CallStackFrame::CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments)
+   APValue *Arguments, const APValue *CArgs)
 : Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
-  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
+  Arguments(Arguments), ConstantArgs(CArgs), CallLoc(CallLoc),
+  Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }
@@ -1679,7 +1686,7 @@
   Out << ", ";
 
 const ParmVarDecl *Param = *I;
-const APValue  = Frame->Arguments[ArgIndex];
+const APValue  = Frame->ConstantArgs[ArgIndex];
 Arg.printPretty(Out, Frame->Info.Ctx, Param->getType());
 
 if (ArgIndex == 0 && IsMemberCall)
@@ -4455,7 +4462,11 @@
   if (!Info.CheckCallLimit(CallLoc))
 return false;
 
-  CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data());
+  // ArgValues - may be modified by the called function.
+  // ConstantArgs - can't be modified by the called function
+  const ArgVector ConstantArgs(ArgValues.begin(), ArgValues.end());
+  CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data(),
+   ConstantArgs.data());
 
   // For a trivial copy or move assignment, perform an APValue copy. This is
   // essential for unions, where 

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-11 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:486
+/// diagnostics
+ConstArgs ConstantArgs;
+

Please don't use a typedef for this. Follow the style of the surrounding lines.

const APValue *ConstantArgs;

(The pointer member itself should not be `const`-qualified. Compare to `const 
LValue *This;` two declarations above.)



Comment at: clang/lib/AST/ExprConstant.cpp:4520
   APValue *ArgValues,
+  APValue const *const ConstantArgs,
   const CXXConstructorDecl *Definition,

`const APValue *ConstantArgs,`



Comment at: clang/lib/AST/ExprConstant.cpp:4684
 
-  return HandleConstructorCall(E, This, ArgValues.data(), Definition,
-   Info, Result);
+  ArgVector ConstantArg(ArgValues.begin(), ArgValues.end());
+  return HandleConstructorCall(E, This, ArgValues.data(), ConstantArg.data(),

Surely you meant `ConstantArgs` (plural) here.



Comment at: clang/test/SemaCXX/constant-expression-cxx1y.cpp:1183
+// expected-note@+1 {{'f2(0)}}
+constexpr int a = f2(0);

Personally, I would prefer to see something as simple as my original example; I 
don't think the extra 6 levels of recursion here makes the test any easier to 
understand.
```
constexpr int f(int i) {
i = -i;
return 1 << i;  // expected-note{{negative shift count -1}}
}
static_assert(f(1));
// expected-error@-1{{static_assert expression is not an integral constant 
expression}}
// expected-note@-2{{in call to 'f(1)'}}
```

Also, procedural note: I expect you'll be asked to reupload this diff with full 
context (`git diff -U999 ...`).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60561/new/

https://reviews.llvm.org/D60561



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

2019-04-11 Thread Gauthier via Phabricator via cfe-commits
Tyker created this revision.
Tyker added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

this is a bugfix for this 

added a constant copy of argument to CallStackFrame.
and use it in describeCall instead of the modifiable version od arguments.
added test for the diagnostic


Repository:
  rC Clang

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,25 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i) {
+  i -= 1;
+  return 1 << i; // expected-note {{negative shift count -8}}
+}
+
+constexpr int f2(int i) {
+  i -= 1;
+  if (i < -6)
+return f1(i); // expected-note {{'f1(-7)}}
+  // expected-note@+6 {{'f2(-6)}}
+  // expected-note@+5 {{'f2(-5)}}
+  // expected-note@+4 {{'f2(-4)}}
+  // expected-note@+3 {{'f2(-3)}}
+  // expected-note@+2 {{'f2(-2)}}
+  // expected-note@+1 {{'f2(-1)}}
+  return f2(i);
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{'f2(0)}}
+constexpr int a = f2(0);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -476,9 +476,15 @@
 const LValue *This;
 
 /// Arguments - Parameter bindings for this function call, indexed by
-/// parameters' function scope indices.
+/// parameters' function scope indices. may be modified by called function
 APValue *Arguments;
 
+using ConstArgs = APValue const *const;
+/// ConstantArgs - Parameter bindings for this function call, indexed by
+/// parameters' function scope indices. can't be modified. used for
+/// diagnostics
+ConstArgs ConstantArgs;
+
 // Note that we intentionally use std::map here so that references to
 // values are stable.
 typedef std::pair MapKeyTy;
@@ -519,7 +525,7 @@
 
 CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments);
+   APValue *Arguments, ConstArgs ConstantArgs);
 ~CallStackFrame();
 
 // Return the temporary for Key whose version number is Version.
@@ -789,14 +795,15 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext , Expr::EvalStatus , EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr,
+  nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue ) {
   EvaluatingDecl = Base;
@@ -1235,9 +1242,10 @@
 
 CallStackFrame::CallStackFrame(EvalInfo , SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments)
+   APValue *Arguments, ConstArgs CArgs)
 : Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
-  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
+  Arguments(Arguments), ConstantArgs(CArgs), CallLoc(CallLoc),
+  Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }
@@ -1679,7 +1687,7 @@
   Out << ", ";
 
 const ParmVarDecl *Param = *I;
-const APValue  = Frame->Arguments[ArgIndex];
+const APValue  = Frame->ConstantArgs[ArgIndex];
 Arg.printPretty(Out, Frame->Info.Ctx, Param->getType());
 
 if (ArgIndex == 0 && IsMemberCall)
@@ -4455,7 +4463,11 @@
   if (!Info.CheckCallLimit(CallLoc))
 return false;
 
-  CallStackFrame Frame(Info, CallLoc, Callee, This,