sammccall added a comment.
I agree we often show the wrong amount of sugar, and with the thrust of this
patch.
Some pareto-improments are possible with a single type, but we can't solve this
problem satisfactorily:
- the right amount of sugar can vary for the same code pattern
(`vector<T>::iterator` vs `cast_retty<x, y>::ret_type`)
- it can depend on what the reader's context is, which we can't know
However I think desugaring all the way to the canonical type, and showing it
whenever the strings are not exactly equal, are too blunt as policies.
A similar situation in clang is deciding when to print "aka" in a diagnostic,
and what the AKA type should be. The code controlling that is in `Desugar()` in
`clang/lib/AST/ASTDiagnostic.cpp`. It's handled pretty carefully, and in a
place that's likely to be well-maintained.
(It's also wrapped in functions that process a whole diagnostic to take into
account multiple types that appear, which we can ignore).
I'd suggest we expose that function from ASTDiagnostic.h and use it for this
purpose.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:691
} else {
HI.Definition = printType(QT, PP);
----------------
This seems like a pretty important case to handle.
A wrinkle is that this gets treated as C++ code (e.g. client-side syntax
highlighted).
So we might need something like
```
int64_t
// aka
long long
```
or
```
int64_t
// aka: long long
```
================
Comment at: clang-tools-extra/clangd/Hover.cpp:1076
+ if (CanonicalReturnType && CanonicalReturnType != ReturnType)
+ ReturnTypeStr += llvm::formatv(" (aka '{0}')", *CanonicalReturnType);
+ Output.addParagraph().appendText("→ ").appendCode(ReturnTypeStr);
----------------
I'm not sure that quoting the aka type when we don't quote the original is
appropriate
================
Comment at: clang-tools-extra/clangd/Hover.cpp:1129
+ CalleeArgInfo->CanonicalType != CalleeArgInfo->Type)
+ ConvertedTypeStr +=
+ llvm::formatv(" aka '{0}'", *CalleeArgInfo->CanonicalType);
----------------
I think printing two types here is too disruptive to the reading flow, this is
a parenthetical already
================
Comment at: clang-tools-extra/clangd/Hover.h:35
+ /// CanonicalType
+ llvm::Optional<std::string> CanonicalType;
/// None for unnamed parameters.
----------------
instead of these ad-hoc pairs, I'd suggest introducing a common struct like:
```
Optional<PrintedType> Type;
struct PrintedType {
std::string Type;
Optional<std::string> AKA;
};
```
that way producing function like printType and formatting functions that
produce "foo (aka bar)" can more simply be shared between instances of this
pattern
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114522/new/
https://reviews.llvm.org/D114522
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
