[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

Wed Feb 22 11:18:18 PST 2023

cor3ntin added inline comments.

================
Comment at: clang/lib/Sema/SemaExpr.cpp:19113
+    // parameter appertaining to the same declaration as that attribute.
+    if(auto* Parm = dyn_cast<ParmVarDecl>(Var); Parm && Parm->getDeclContext() == DC)
+      return true;
----------------
aaron.ballman wrote:
> Formatting nit.
> 
> I have a hazy recollection that we sometimes have a weird declaration context for parameters where the parameter doesn't think it's associated with a function DC but instead with the translation unit DC. I seem to recall this having something to do with PCH/AST importing, but I can't seem to find any open issues about it. So this might be fine, but it may have some weird edge cases still.
Yep, parameters are all created in the top level dc before being attached to the function. but by the time they get to be captured by a lambda they are attached to the function
(default parameters initializers can't capture)

================
Comment at: clang/test/SemaCXX/lambda-expressions.cpp:675-676
+StringLiteral(const char (&array)[N])
+    __attribute__((enable_if(__builtin_strlen(array) == 2,
+                              "invalid string literal")));
+};
----------------
aaron.ballman wrote:
> 1) This code already compiles today without issue (https://godbolt.org/z/q6hPxoWq9), so are you sure this is testing what needs to be tested?
> 
> 2) What about for non-GNU-style attributes ([[]] attributes that appertain to the function type)? e.g.,
> ```
> template <int N>
> void foo(const char (&array)[N]) [[clang::annotate_type("test", array)]];
> ```
Yes, this is something that was broken by this patch.
enable_if did create odr uses (incorrectly). I can add a test for annotate_type but I don't think it would run into the same issue at all. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351