nridge added inline comments.
================
Comment at: clang-tools-extra/clangd/AST.cpp:545
+ // TemplateTypeParmTypes for implicit TTPs, instead of AutoTypes.
+ // Also we don't look very hard, just stripping const, references, pointers.
+ static TemplateTypeParmTypeLoc findContainedAutoTTPLoc(TypeLoc TL) {
----------------
sammccall wrote:
> nridge wrote:
> > Maybe call out an example like `vector<auto>` as a future nice-to-handle?
> Done. Note that clang currently rejects `vector<auto>`, AFAICT it's valid
> though.
Huh, on second look, it appears that `vector<auto>` is a feature that was
dropped when merging the Concepts TS into C++20 (at least, I don't see anything
in http://eel.is/c++draft/dcl.spec.auto that would allow it). GCC appears to
support it as an extension.
================
Comment at: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp:88
+ StartsWith("fail: Could not deduce"));
+ EXPECT_EQ(apply("auto X = [](^auto){return 0;}; int Y = X(42);"),
+ "auto X = [](int){return 0;}; int Y = X(42);");
----------------
sammccall wrote:
> nridge wrote:
> > Maybe add a conflicting-deduction case? (I know the GetDeducedType test
> > already has one, but here it would be especially wrong.)
> >
> > Also, should we perhaps disable this in header files (maybe excepting
> > function-local symbols), since an including source file could have
> > additional instantiations?
> > Maybe add a conflicting-deduction case? (I know the GetDeducedType test
> > already has one, but here it would be especially wrong.)
> Done.
>
> > Also, should we perhaps disable this in header files (maybe excepting
> > function-local symbols), since an including source file could have
> > additional instantiations?
>
> Disabling features in headers seems sad. Detecting when the function is local
> doesn't seem like a great solution to this:
> - it's going to be hard to detect the common ways functions are hidden
> within headers (`detail` namespaces, inaccessible members, ...)
> - it's still going to be wrong when some instantiations come via (local)
> templates that are themselves instantiated from the including file
>
> I suspect in most cases where multiple instantiations are intended we're
> going to see either no instantiations or many. Can we try it and see if there
> are false positive complaints?
To clarify, I meant excepting only cases like this:
```
// header.hpp
void foo(std::function<void(ConcreteType)> callback);
void bar() {
// We know this will only be instantiated with ConceteType,
// regardless of what an including source file does
foo([](auto x){});
}
```
but I'm sympathetic to the argument that neither false-positives (we offer a
refactoring that breaks code) nor false-negative (we fail to offer a
refactoring the user wants to make) are super impactful here and so we should
do the easier thing (i.e. not try to restrict at all).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119537/new/
https://reviews.llvm.org/D119537
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
