sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp:88
+      R"cpp(template <typename T> concept C = true;
+^C a^uto abc();
+    )cpp");
----------------
hokein wrote:
> sammccall wrote:
> > This shouldn't be unavailable because it's constrained, it should be 
> > unavailable because it's not deduced.
> > 
> > Does this case pass or fail today?
> > 
> > If the issue with constrained auto is syntactic, can you add a deductible 
> > test case?
> > it should be unavailable because it's not deduced.
> 
> This is ideal, but not the behavior today -- the tweak is available but being 
> failed to apply as we don't check the auto is deduced during prepare stage 
> (it is expensive, requiring an AST traversal). 
> 
> For deducible cases, we replace the `C auto` with the actual type, e.g.
> ```
> template <typename T> concept C = true;
> C auto var = 123;  // => int var = 123;
> ```
> I don't think this is an expected behavior. Given the deductible & 
> nondeductible cases, it seems like an improvement to disable the tweak for 
> constrained auto.
> > it should be unavailable because it's not deduced.
> 
> This is ideal, but not the behavior today -- the tweak is available but being 
> failed to apply as we don't check the auto is deduced during prepare stage 
> (it is expensive, requiring an AST traversal). 

Sure, but this doesn't have anything to do with it being constrained.
We offer the tweak on `auto x();` too, and equally shouldn't.

Meanwhile we offer the tweak on `auto x() { return 42; }` and it works 
correctly, and we should also offer it on `Integer auto x() { return 42; }`.


> For deducible cases, we replace the `C auto` with the actual type, e.g.
> ```
> template <typename T> concept C = true;
> C auto var = 123;  // => int var = 123;
> ```
> I don't think this is an expected behavior. 

That's exactly what I'd expect this action to do. What would you expect it to 
do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117463

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to