[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

1594

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

9

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!

1594

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

9

Thanks! See my comment above.

[shafik]

clang/lib/Sema/SemaTemplate.cpp

1594

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

13

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

13

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

13

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

13

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 <iostream> // massberg
 #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"
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()) {
+          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 auto z = 2;
+
+auto f() {
+  const auto x = 1;
+
+  auto lambda1 = [] <auto y = x> {}; // expected-error {{default argument references local variable x of enclosing function}}
+  auto lambda2 = [] <auto y = 42> {};
+  auto lambda3 = [] <auto y = z> {};
+  auto lambda4 = [] <auto y> {};
+}
+}