[clang] Show error when a local variables is passed as default template parameter

[massberg]

Summary

I haven't found an existing error message for this in
clang/include/clang/Basic/DiagnosticSemaKinds.td so I have added a new
one. I'm not sure if it in the right place and the formulation might be
improved.

Fixes #48755

[massberg]

Fix test.

[shafik]

It looks like we have an existing message for the regular function case e.g.:

void g() {
    int x;
    void f(int y = x);

    auto lambda1 = [] <auto y = x> {}; 
}

The diagnostic is :

error: default argument references local variable 'x' of enclosing function
    void f(int y = x);
                   ^

see godbolt: https://godbolt.org/z/1vneM5b35

[shafik]

It looks like the existing diagnostic is issued by CheckDefaultArgumentVisitor. I think you can reuse it here but not 100% on that.

[massberg]

In D139400#3974654, @shafik wrote:

It looks like the existing diagnostic is issued by CheckDefaultArgumentVisitor. I think you can reuse it here but not 100% on that.

Thanks for the pointer! I will have a look.

[massberg]

Use isLocalVarDecl to check if we have a local variable.

[massberg]

Reuse existing error message.

[massberg]

In D139400#3974624, @shafik wrote:

It looks like we have an existing message for the regular function case

Thanks! We can indeed reuse this error message.

In D139400#3974654, @shafik wrote:

It looks like the existing diagnostic is issued by CheckDefaultArgumentVisitor. I think you can reuse it here but not 100% on that.

Thanks for the pointer! CheckDefaultArgumentVisitor is for default arguments of functions. As it does many additional checks we cannot reuse it for testing default arguments of templates.

[shafik]

clang/lib/Sema/SemaTemplate.cpp

1593

So if we look at CheckDefaultArgumentVisitor::VistDeclRefExpr it has a comment about having to check ODR use due to cwg 2346 and I believe you should be using that as well.

Which would make your example below pass since the variable is const see: https://godbolt.org/z/78Tojh6q5

It looks like neither gcc nor MSVC implement this DR.

[shafik]

clang/test/SemaTemplate/default-template-arguments.cpp

10

To clarify my comment about, I think w/ the ODR use checking this should pass.

[shafik]

clang/lib/Sema/SemaTemplate.cpp

11

Please remove.

[massberg]

Additionally check for ODR use and remove inlcude added by accident.

[massberg]

clang/lib/Sema/SemaTemplate.cpp

11

Ups, thanks!

1593

Thanks! I have extended the check.

However, the error is still shown for the const case in the test.
When comparing with your function example, I observed that the difference is that cone an int and otherwise an auto is used.
I haven't expected a difference, but with auto the existing check for local variables as parameters of functions shows an error in case of
auto instead of int: https://godbolt.org/z/Kvfvn4aEr (Is this a bug?).

I have extended the test cases to have cases with auto and cases with int, but even in the int case the code still prints an error.

clang/test/SemaTemplate/default-template-arguments.cpp

10

Thanks! See my comment above.

[shafik]

clang/lib/Sema/SemaTemplate.cpp

1593

I am bit surprised that they are considered ODR used in the case you added to the test. I am reading the standard and I don't see it but I need to dig some more to figure out if I am missing something.

[tahonermann]

clang/test/SemaTemplate/default-template-arguments.cpp

12

MSVC and Clang currently accept x_static. The resolution of CWG 2346 suggests that this case (and maybe x_constexpr) should be accepted following the adoption of P0588R1 (Simplifying implicit lambda capture). However, it isn't obvious to me that P0588R1 actually addresses this. It might be worth following up with CWG to understand what the intent is.

[shafik]

clang/test/SemaTemplate/default-template-arguments.cpp

12

I posted to the core reflector on this topic and there is already some feedback but let me wait to see if there is a consensus of this will need further discussions in core.

[massberg]

clang/test/SemaTemplate/default-template-arguments.cpp

12

Hi! Is there any update on this? How should I proceed?

[massberg]

Any advice how I should proceed here?

[shafik]

clang/test/SemaTemplate/default-template-arguments.cpp

12

I started the discussion here: https://lists.isocpp.org/core/2022/12/13642.php and there were a lot of points brought up. I think a core issue should have been opened but it does not look like it was.
I am following up now.

clang/lib/Sema/SemaTemplate.cpp

 //===------- SemaTemplate.cpp - Semantic Analysis for C++ Templates -------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //===----------------------------------------------------------------------===//
 //
 //  This file implements semantic analysis for C++ templates.
 //===----------------------------------------------------------------------===//
 
 #include "TreeTransform.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
NamedDecl *Sema::ActOnNonTypeTemplateParameter(Scope *S, Declarator &D,
   //   template-parameter that is not a template parameter pack.
   if (Default && IsParameterPack) {
     Diag(EqualLoc, diag::err_template_param_pack_default_arg);
     Default = nullptr;
   }
 
   // Check the well-formedness of the default template argument, if provided.
   if (Default) {
+    // Local variables may not be used as default template arguments.
+    if (auto *DRE = dyn_cast<DeclRefExpr>(Default)) {
+      if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+        if (VD->isLocalVarDecl() && !DRE->isNonOdrUse()) {
+          Diag(DRE->getLocation(),
+               diag::err_param_default_argument_references_local)
+              << VD->getNameAsString();
+        }
+      }
+    }
+
     // Check for unexpanded parameter packs.
     if (DiagnoseUnexpandedParameterPack(Default, UPPC_DefaultArgument))
       return Param;
 
     TemplateArgument SugaredConverted, CanonicalConverted;
     ExprResult DefaultRes = CheckTemplateArgument(
         Param, Param->getType(), Default, SugaredConverted, CanonicalConverted,
         CTAK_Specified);

clang/test/SemaTemplate/default-template-arguments.cpp

+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+
+namespace GH48755 {
+constexpr int z = 2;
+
+auto f() {
+  constexpr int x_constexpr = 1;
+  const int x_const = 1;
+  static int x_static = 1;
+
+  auto lambda1 = [] <auto y = x_constexpr> {}; // expected-error {{default argument references local variable x_constexpr of enclosing function}}
+  auto lambda2 = [] <auto y = x_static> {}; // expected-error {{default argument references local variable x_static of enclosing function}}
+  auto lambda3 = [] <auto y = x_const> {}; // expected-error {{default argument references local variable x_const of enclosing function}}
+  auto lambda4 = [] <auto y = 42> {};
+  auto lambda5 = [] <auto y = z> {};
+  auto lambda6 = [] <auto y> {};
+
+  const int x_int_const = 1;
+  auto lambda7 = [] <int y = x_int_const> {}; // expected-error {{default argument references local variable x_int_const of enclosing function}}
+}
+}