hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:663
+ const auto *Var = dyn_cast<VarDecl>(D);
+ if (Var && !llvm::isa<ParmVarDecl>(Var)) {
if (const Expr *Init = Var->getInit())
----------------
We're ignoring all `ParmVarDecl` cases,
The crash here is for broken code. For the crash case, the AST node looks like
```
`-ParmVarDecl 0x563b7cc853c0 <col:10, col:20> col:14 invalid param 'Foo':'Foo'
cinit
`-OpaqueValueExpr 0x563b7cc854a0 <col:20> 'Foo':'Foo'
```
One alternative is to exclude invalid `VarDecl`, then the Hover feature still
keep working on valid `ParmVarDecl`.
```
if (const auto* Var = dyn_cast<VarDecl>(D); Var && !Var->isInvalidDecl()) {
...
}
```
================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:3726
+TEST(Hover, FunctionParameterDefaulValueNotEvaluated) {
+ Annotations T("void foo(int p^aram = 5);");
+ TestTU TU = TestTU::withCode(T.code());
----------------
VitaNuo wrote:
> hokein wrote:
> > I believe this case should trigger a crash, but I don't see the crash with
> > a trunk-built clangd, do we miss something here?
> It crashes on invalid code. I've inspected two user workspaces:
> 1.
>
> ```
> class Foo{};
> void foo(Foo param = nullptr);
> ```
>
> 2.
> ```
> void foo(ClassName param =
> functionReturningObjectOfSimilarSoundingButUnrelatedClass());
> ```
>
> I guess `clangd` is not even expected to act soundly in the face of
> non-compiling code.
> But since these crashes are easy to get rid of, and evaluating parameters in
> function declarations is pointless anyways, we can just avoid these crashes
> at no extra cost. I cannot use non-compiling code in a hover test, though.
thanks for the clarification. That makes sense.
We can still use these broken code (the first one should be good enough) in the
HoverTest, we need to add a magic comment `/* error-ok */`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153015/new/
https://reviews.llvm.org/D153015
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
