[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter
Jens Massberg via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 12 08:34:55 PST 2022
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
More information about the cfe-commits
mailing list