Page MenuHomePhabricator

[clangd] Show correct hover tooltip for non-preamble macro definition.
AbandonedPublic

Authored by ilya-golovenko on Aug 4 2020, 3:35 PM.

Details

Summary

Incorrect definition is shown in tooltip when hovering over a non-preamble macro definition.
In the example below the definition will have 'namespace ns {}' instead of correct '#define FOO 1'.

namespace ns {
#define [[FO^O]] 1
}

Diff Detail

Event Timeline

ilya-golovenko created this revision.Aug 4 2020, 3:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2020, 3:35 PM
ilya-golovenko requested review of this revision.Aug 4 2020, 3:35 PM

I'm not quite sure my fix is the best way to fix the issue, so any advices are appreciated.

ilya-golovenko edited the summary of this revision. (Show Details)Aug 4 2020, 3:39 PM

Sigh, (I think) this is working for macros defined in preamble region as a side effect of preamble being loaded separately and before anything in the main file. E.g.

#define FO^O 1
#define FOO 2

void foo() { int x = FOO; };

hovering over the first definition of FOO (at carrot) will yield #define FOO 2, because that's the latest definition before main file.

That being said, I don't really think there's much value in showing the hovercard at definition of a macro, as that's what we show anyways. (showing the surrounding namespace is even worse, but that's a different problem and i thought we've solved it recently, are you sure you are seeing that behaviour with a build from head?).
So I believe you are rather trying to fix the issue of displaying misleading hover info, and if that's the case I think we should rather fix that by not displaying hover infos at the macro definition locations.

WDYT?

I agree the fix is not correct. I will check how it currently works with clangd built from master branch.

I'm not quite sure there should be no tooltip for a #define. Such tooltip is not really useful, but at the same time clangd shows tooltips for function declarations/defnitions, namespaces, variables, etc. and not showing it for macro definition will be a little bit inconsistent from my point of view.

I'm not quite sure there should be no tooltip for a #define. Such tooltip is not really useful, but at the same time clangd shows tooltips for function declarations/defnitions, namespaces, variables, etc. and not showing it for macro definition will be a little bit inconsistent from my point of view.

We actually had a couple users saying those are redundant too. I agree with them mostly, i suppose these are only useful because we talk about the associated namespace/class/accesspecifer. E.g. you can easily figure out what's the class owning a field, and whether it is private/public without worrying traversing upwards.
Who knows, maybe we should only be displaying that information on the declaration/definition of symbols rather than all the stuff that's already probably next to user's cursor (I say probably because you can have declarations coming from macro invocations).

ilya-golovenko abandoned this revision.Sat, Sep 12, 5:15 AM