Implements CWG 2082 Referring to parameters in unevaluated operands of default arguments

[Mordante]

Summary

This implements the current standard wording for [dcl.fct.default]p9 and [dcl.fct.default]p7. This has been changed by CWG 2082.

Note: I don't have access to the paper therefore I assume it retroactively applies to all C++ standards.

[rsmith]

clang/lib/Sema/SemaDeclCXX.cpp

100–101

Please add tests for the distinction between "potentially-evaluated" and "odr-used" here, for example:

void f() {
  const int n = 123;
  void g(int k = n); // ok, not an odr-use
}

clang/www/cxx_dr_status.html

3 ↗

(On Diff #213186)

Note that this is an auto-generated file. To update it, you need to add a test to the relevant file (test/CXX/drs/dr20xx.cpp) with a suitable comment (// dr2082: 10 to mark this implemented in Clang 10), grab a recent cwg_index.html file, and run the make_cxx_dr_status script.

[Mordante]

clang/lib/Sema/SemaDeclCXX.cpp

100–101

I added the test but unfortunately clang disagrees with you and considers n ODR used.
I'll have look how to teach clang n is not ODR used.

clang/www/cxx_dr_status.html

3 ↗

(On Diff #213186)

Thanks for the info, I'll have a look at it after I fix the ODR used part.

[Mordante]

Updated the unit tests as requested. This required the Sema::ActOnParamDefaultArgument to delay a part of the ODR validation until the default argument has been 'instantiated'.
As discussed on IRC; the up to date cwg_index.html is not public, so I only updated the unit test and removed the changes to cxx_dr_status.html.

[rsmith]

clang/lib/Sema/SemaDeclCXX.cpp

143–148

Instead of visiting the default argument twice, can we call SetParamDefaultArgument first before doing the checks?

clang/lib/Sema/SemaDeclCXX.cpp

namespace {
 
   /// VisitDeclRefExpr - Visit a reference to a declaration, to
   /// determine whether this declaration can be used in the default
   /// argument expression.
   bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(DeclRefExpr *DRE) {
     NamedDecl *Decl = DRE->getDecl();
     if (ParmVarDecl *Param = dyn_cast<ParmVarDecl>(Decl)) {
       // C++ [dcl.fct.default]p9
-      //   Default arguments are evaluated each time the function is
-      //   called. The order of evaluation of function arguments is
-      //   unspecified. Consequently, parameters of a function shall not
-      //   be used in default argument expressions, even if they are not
-      //   evaluated. Parameters of a function declared before a default
-      //   argument expression are in scope and can hide namespace and
-      //   class member names.
+      //   A default argument is evaluated each time the function is called
+      //   with no argument for the corresponding parameter. A parameter shall
+      //   not appear as a potentially-evaluated expression in a default
+      //   argument. Parameters of a function declared before a default
+      //   argument are in scope and can hide namespace and class member
+      //   names.
+      if (DRE->isNonOdrUse() == NOUR_Unevaluated)
+        return false;
+
       return S->Diag(DRE->getBeginLoc(),
                      diag::err_param_default_argument_references_param)
              << Param->getDeclName() << DefaultArg->getSourceRange();
-    } else if (VarDecl *VDecl = dyn_cast<VarDecl>(Decl)) {
-      // C++ [dcl.fct.default]p7
-      //   Local variables shall not be used in default argument
-      //   expressions.
-      if (VDecl->isLocalVarDecl())
-        return S->Diag(DRE->getBeginLoc(),
-                       diag::err_param_default_argument_references_local)
-               << VDecl->getDeclName() << DefaultArg->getSourceRange();
     }
 
     return false;
   }
-
   /// VisitCXXThisExpr - Visit a C++ "this" expression.
   bool CheckDefaultArgumentVisitor::VisitCXXThisExpr(CXXThisExpr *ThisE) {
     // C++ [dcl.fct.default]p8:
     //   The keyword this shall not be used in a default argument of a
     //   member function.
     return S->Diag(ThisE->getBeginLoc(),
                    diag::err_param_default_argument_references_this)
            << ThisE->getSourceRange();
bool CheckDefaultArgumentVisitor::VisitLambdaExpr(LambdaExpr *Lambda) {
     //   implicitly or explicitly capture any entity.
     if (Lambda->capture_begin() == Lambda->capture_end())
       return false;
 
     return S->Diag(Lambda->getBeginLoc(), diag::err_lambda_capture_default_arg);
   }
 }
 
+  /// CheckDefaultArgumentVisitorODR - C++ [dcl.fct.default] Traverses
+  /// the default argument of a parameter to determine whether it
+  /// contains ODR violations. These violations cannot be checked in
+  /// \ref CheckDefaultArgumentVisitor since the DeclRefExp's may be changed to
+  /// an implicit cast from an LValue to RValue by \ref SetParamDefaultArgument.
+  /// When that happens the ODR usage may be allowed.
+  class CheckDefaultArgumentVisitorODR
+    : public StmtVisitor<CheckDefaultArgumentVisitorODR, bool> {
+    Expr *DefaultArg;
+    Sema *S;
+
+  public:
+    CheckDefaultArgumentVisitorODR(Expr *defarg, Sema *s)
+        : DefaultArg(defarg), S(s) {}
+
+    bool VisitExpr(Expr *Node);
+    bool VisitDeclRefExpr(DeclRefExpr *DRE);
+  };
+
+  /// VisitExpr - Visit all of the children of this expression.
+  bool CheckDefaultArgumentVisitorODR::VisitExpr(Expr *Node) {
+    bool IsInvalid = false;
+    for (Stmt *SubStmt : Node->children())
+      IsInvalid |= Visit(SubStmt);
+    return IsInvalid;
+  }
+
+  /// VisitDeclRefExpr - Visit a reference to a declaration, to determine
+  /// whether this declaration can be used in the default argument expression.
+  bool CheckDefaultArgumentVisitorODR::VisitDeclRefExpr(DeclRefExpr *DRE) {
+    NamedDecl *Decl = DRE->getDecl();
+
+    if (VarDecl *VDecl = dyn_cast<VarDecl>(Decl)) {
+      // C++ [dcl.fct.default]p7
+      //   A local variable cannot be odr-used (6.2) in a default argument.
+      // C++2a [basic.def.odr]p4:
+      //   A variable x whose name appears as a potentially-evalauted expression
+      //   e is odr-used by e unless [...]
+      //   -- x is a variable of non-reference type that is usable in constant
+      //      expressions and has no mutable subobjects, and e is an element of
+      //      the set of potential results of an expression of
+      //      non-volatile-qualified non-class type to which the
+      //      lvalue-to-rvalue conversion is applied, or
+      if (DRE->isNonOdrUse() != NOUR_None)
+        return false;
+
+      if (VDecl->isLocalVarDecl())
+        return S->Diag(DRE->getBeginLoc(),
+                       diag::err_param_default_argument_references_local)
+               << VDecl->getDeclName() << DefaultArg->getSourceRange();
+    }
+    return false;
+  }
+
 void
 Sema::ImplicitExceptionSpecification::CalledDecl(SourceLocation CallLoc,
                                                  const CXXMethodDecl *Method) {
   // If we have an MSAny spec already, don't bother.
   if (!Method || ComputedEST == EST_MSAny)
     return;
 
   const FunctionProtoType *Proto
Sema::ActOnParamDefaultArgument(Decl *param, SourceLocation EqualLoc,
 
   // Check that the default argument is well-formed
   CheckDefaultArgumentVisitor DefaultArgChecker(DefaultArg, this);
   if (DefaultArgChecker.Visit(DefaultArg)) {
     Param->setInvalidDecl();
     return;
   }
 
-  SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
+  if (!SetParamDefaultArgument(Param, DefaultArg, EqualLoc)) {
+    CheckDefaultArgumentVisitorODR DefaultArgCheckerODR(DefaultArg, this);
+    if (DefaultArgCheckerODR.Visit(Param->getDefaultArg()))
+      Param->setInvalidDecl();
+  }
 }
 
 /// ActOnParamUnparsedDefaultArgument - We've seen a default
 /// argument for a function parameter, but we can't parse it yet
 /// because we're inside a class definition. Note that this default
 /// argument will be parsed later.
 void Sema::ActOnParamUnparsedDefaultArgument(Decl *param,
                                              SourceLocation EqualLoc,

clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p9.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int a;
+int f(int a, int b = a); // expected-error {{default argument references parameter 'a'}}
+typedef int I;
+int g(float I, int b = I(2)); // expected-error {{called object type 'float' is not a function or function pointer}}
+int h(int a, int b = sizeof(a));
+
+int b;
+class X {
+  int a;
+  int mem1(int i = a); // expected-error {{invalid use of non-static data member 'a'}}
+  int mem2(int i = b);
+  static int b;
+};
+
+int f(int = 0);
+void h() {
+  int j = f(1);
+  int k = f();
+}
+int (*p1)(int) = &f;
+int (*p2)() = &f; // expected-error {{cannot initialize a variable of type 'int (*)()' with an rvalue of type 'int (*)(int)': different number of parameters (0 vs 1)}}

clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-void h()
+void f()
 {
   int i;
-  extern void h2(int x = sizeof(i)); // expected-error {{default argument references local variable 'i' of enclosing function}}
+  extern void g(int x = i); // expected-error {{default argument references local variable 'i' of enclosing function}}
+  extern void h(int x = sizeof(i));
 }

clang/test/CXX/drs/dr20xx.cpp

 // RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors \
 // RUN:            -Wno-variadic-macros -Wno-c11-extensions
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
 // RUN: %clang_cc1 -std=c++1z -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
 
 #if __cplusplus < 201103L
 #define static_assert(...) _Static_assert(__VA_ARGS__)
 #endif
 
+
+namespace dr2082 { // dr2082: 10
+namespace local_var {
+void g() {
+  int k = 42;
+  void l(int m = k); // expected-error {{default argument references local variable 'k' of enclosing function}}
+}
+} // namespace local_var
+namespace local_const {
+void g() {
+  const int k = 42;
+  void l(int m = k);
+}
+} // namespace local_const
+#if __cplusplus >= 201103L
+namespace local_constexpr {
+void g() {
+  constexpr int k = 42;
+  void l(int m = k);
+}
+} // namespace local_constexpr
+#endif
+
+namespace local_const_float_to_integral {
+void g() {
+  const double k = 42;
+  void l(int m = k); // expected-error {{default argument references local variable 'k' of enclosing function}}
+}
+} // namespace local_const_float_to_integral
+#if __cplusplus >= 201103L
+namespace local_constexpr_float_to_integral {
+void g() {
+  constexpr double k = 42;
+  void l(int m = k);
+}
+} // namespace local_constexpr_float_to_integral
+
+namespace local_member_const {
+struct a {
+  int b;
+  int c;
+};
+void g() {
+  const a n{42, 42};
+  void l(int m = n.b); // expected-error {{default argument references local variable 'n' of enclosing function}}
+}
+} // namespace local_member_const
+namespace local_member_constexpr {
+struct a {
+  int b;
+  int c;
+};
+void g() {
+  constexpr a n{42, 42};
+  void l(int m = n.b);
+}
+} // namespace local_member_constexpr
+namespace local_member_mutable {
+struct a {
+  int b;
+  mutable int c;
+};
+void g() {
+  constexpr a n{42, 42};
+  void l(int m = n.b); // expected-error {{default argument references local variable 'n' of enclosing function}}
+}
+} // namespace local_member_mutable
+#endif
+}
+
 namespace dr2083 { // dr2083: partial
 #if __cplusplus >= 201103L
   void non_const_mem_ptr() {
     struct A {
       int x;
       int y;
     };
     constexpr A a = {1, 2};