[PATCH] D117463: [clangd] Disable expand-auto action on constrained auto.

[Sam McCall via Phabricator via cfe-commits]

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