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

[Jens Massberg via Phabricator via cfe-commits]

massberg marked 2 inline comments as done.
massberg added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:11
 
+#include <iostream> // massberg
 #include "TreeTransform.h"
----------------
shafik wrote:
> Please remove.
Ups, thanks!


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:1594
+      if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+        if (VD->isLocalVarDecl()) {
+          Diag(DRE->getLocation(),
----------------
shafik wrote:
> So if we look at `CheckDefaultArgumentVisitor::VistDeclRefExpr` it has a comment about having to check ODR use due to [cwg 2346](https://cplusplus.github.io/CWG/issues/2346.html) 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.
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.


================
Comment at: clang/test/SemaTemplate/default-template-arguments.cpp:9
+
+  auto lambda1 = [] <auto y = x> {}; // expected-error {{default argument references local variable x of enclosing function}}
+  auto lambda2 = [] <auto y = 42> {};
----------------
shafik wrote:
> To clarify my comment about, I think w/ the ODR use checking this should pass.
Thanks! See my comment above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139400