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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:1594
+      if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+        if (VD->isLocalVarDecl()) {
+          Diag(DRE->getLocation(),
----------------
massberg wrote:
> 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.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139400