Skip to content

#define not marked as a definition #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Fomys opened this issue Oct 3, 2024 · 5 comments · Fixed by #344
Closed

#define not marked as a definition #343

Fomys opened this issue Oct 3, 2024 · 5 comments · Fixed by #344
Labels
indexing Related to the index content — missing definitions/references, lexer bugs, new ctags features...

Comments

@Fomys
Copy link
Contributor

Fomys commented Oct 3, 2024

Hello,

I believe there is an issue with the #define parsing in the code here. This definition is not being detected when searching for pm_ptr.

Additionally, just above it, pm_sleep_ptr is not even considered as an identifier, so it cannot be found anywhere.

Same for mmSPI_CONFIG_CNTL_1, at least used here and defined here (BTW on the same page you can see many missing links)

It is also the case for older linux version, I just checked with 6.3 and 5.19, pm_ptr and pm_sleep_ptr are not parsed correctly.

@fstachura
Copy link
Collaborator

It seems that pm_sleep and pm_sleep_ptr are filtered out by this line https://github.com/bootlin/elixir/blob/master/script.sh#L169

root@01eac2eb3583:/usr/local/elixir# ctags -x --kinds-c=+p+x --extras='-{anonymous}' pm.h | grep pm_ptr                                 
pm_ptr           macro       475 pm.h             #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
root@01eac2eb3583:/usr/local/elixir# ctags -x --kinds-c=+p+x --extras='-{anonymous}' pm.h | grep pm_sleep_ptr
pm_sleep_ptr     macro       476 pm.h             #define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))

Notice CONFIG_ in both lines. Same with mmSPI_CONFIG_CNTL_1 and others from that page. All contain CONFIG_.

@tleb
Copy link
Member

tleb commented Oct 4, 2024

One line fix, nice catch @fstachura and @Fomys:

... | grep -avE "^operator |^CONFIG_" | ...

I'll send the fix. Maybe using -e twice if that is supported with the current flags passed.

@fstachura
Copy link
Collaborator

@tleb I have a fix ready, the question is - does filtering identifies that start with CONFIG_ make sense? There are some that are not in Kconfig

@tleb
Copy link
Member

tleb commented Oct 4, 2024

That sounds like a rather unrelated question. Indeed I don't see a reason for filtering those out; they are defines nonetheless that might interest someone someday. Sadly Git log cannot help us: 72571fb.

Attaching the output of the following commands:

⟩ fd -tf | grep -v Kconfig | \
	parallel -L1000 ctags -x 2>/dev/null | \
	grep ^CONFIG_ | sort > /tmp/linux-config-c.txt
⟩ fd -tf | grep -v -e Kconfig -e '\.c$' -e '\.h$' | \
	parallel -L1000 ctags -x 2>/dev/null | \
	grep ^CONFIG_ | sort > /tmp/linux-config-remainder.txt

linux-config-c.txt
linux-config-remainder.txt

For the operator<space> grep, the matches are:

⟩ fd -tf | parallel -L1000 ctags -x 2>/dev/null | grep '^operator '
operator         member      174 kernel/trace/trace_events_hist.c enum field_op_id operator;
operator ->      function     48 tools/testing/selftests/bpf/test_cpp.cpp const T* operator->() const { return skel; }
operator ->      function     50 tools/testing/selftests/bpf/test_cpp.cpp T* operator->() { return skel; }

Let's do that in two steps: first fix the current issue (at least in the code, we don't have to do the full reindex straight away). Then we can discuss about that indexing logic.


edit: related:

elixir/update.py

Lines 318 to 326 in 55921f1

if (db.defs.exists(tok) and
not ( (idx*idx_key_mod + line_num) in defs_idxes and
defs_idxes[idx*idx_key_mod + line_num] == tok ) and
(family != 'M' or tok.startswith(b'CONFIG_'))):
# We only index CONFIG_??? in makefiles
if tok in idents:
idents[tok] += ',' + str(line_num)
else:
idents[tok] = str(line_num)

@fstachura
Copy link
Collaborator

@tleb I think #344 can be merged? It works as a quick fix for this.
Later this filter should be moved to a project specific part of Elixir, so that it's only ran on Linux (and perhaps other projects that use Kconfig with CONFIG_ prefix).
And also, functions that start with operator_ should only be filtered out in C++.

@fstachura fstachura added the indexing Related to the index content — missing definitions/references, lexer bugs, new ctags features... label Feb 21, 2025
@tleb tleb closed this as completed in #344 Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
indexing Related to the index content — missing definitions/references, lexer bugs, new ctags features...
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants