threads / discuss / 53567

git grep --show-function treats GOTO labels as function names

Subject: git grep --show-function treats GOTO labels as function names

## tl;dr

7 messages between May 27, 2020 and May 28, 2020.

replies: 6people: 3as markdown or json

Zach Riggle· May 27, 2020, 22:29 UTC · lore

Git grep is an amazing tool, and --show-function makes it very easy to create reports that contain not only line information, but the containing function as well.

It looks like there is an issue with how the parser handles "goto" labels, as it treats them the same as a function name.

In the example below, there is only one function, with two matches, but using --show-function indicates there are two function names.

$ cat test2.cpp
int main() {
    FOO
exit:
    FOO
}

$ git --version git version 2.26.2

$ git grep --no-index --show-function -e FOO test2.cpp test2.cpp=1=int main() { test2.cpp:2: FOO test2.cpp=3=exit: test2.cpp:4: FOO

Zach Riggle
Jeff King· May 27, 2020, 22:48 UTC · re: Zach Riggle · lore

Re: git grep --show-function treats GOTO labels as function names

On Wed, May 27, 2020 at 05:29:08PM -0500, Zach Riggle wrote:
> It looks like there is an issue with how the parser handles "goto"
> labels, as it treats them the same as a function name.

By default, the function-finding isn't aware of the specific content in the file. But you can associate extensions with particular types, like:

  $ echo '*.cpp diff=cpp' >~/.gitattributes
  $ git config --global core.attributesFile ~/.gitattributes
  $ git grep --no-index --show-function -e FOO test2.cpp
  test2.cpp=int main() {
  test2.cpp:    FOO
  test2.cpp:    FOO

Usually this is done in-repo, but since your example used --no-index, I showed how to set up a per-user attribute file. The "diff" attribute covers both diff and grep (for diff, the hunk headers will also show the function).

The "cpp" diff regexes are built-in to the git binary. We just don't associate any filenames by default. You can also add your own; see the section "Defining a custom hunk-header" from "git help attributes".

-Peff
Zach Riggle· May 27, 2020, 22:54 UTC · re: Jeff King · lore

Re: git grep --show-function treats GOTO labels as function names

Awesome, thanks!
Zach Riggle
On Wed, May 27, 2020 at 5:48 PM Jeff King <peff@peff.net> wrote:
Show 26 quoted lines
>
> On Wed, May 27, 2020 at 05:29:08PM -0500, Zach Riggle wrote:
>
> > It looks like there is an issue with how the parser handles "goto"
> > labels, as it treats them the same as a function name.
>
> By default, the function-finding isn't aware of the specific content in
> the file. But you can associate extensions with particular types, like:
>
>   $ echo '*.cpp diff=cpp' >~/.gitattributes
>   $ git config --global core.attributesFile ~/.gitattributes
>   $ git grep --no-index --show-function -e FOO test2.cpp
>   test2.cpp=int main() {
>   test2.cpp:    FOO
>   test2.cpp:    FOO
>
> Usually this is done in-repo, but since your example used --no-index, I
> showed how to set up a per-user attribute file. The "diff" attribute
> covers both diff and grep (for diff, the hunk headers will also show the
> function).
>
> The "cpp" diff regexes are built-in to the git binary. We just don't
> associate any filenames by default. You can also add your own; see the
> section "Defining a custom hunk-header" from "git help attributes".
>
> -Peff
Zach Riggle· May 27, 2020, 23:04 UTC · re: Zach Riggle · lore

Re: git grep --show-function treats GOTO labels as function names

It looks like that does the trick for "goto" labels, but there are also some issue on function name parsing with attributes when they are split onto a second line.

$ cat attr.cpp
int main() __attribute__ ( (no_sanitize("alignment")) )
{
    FOO
}
$ git grep --no-index --show-function -e FOO attr.cpp
attr.cpp=2=__attribute__ ( (no_sanitize("alignment")) )
attr.cpp:4:    FOO
Zach Riggle
On Wed, May 27, 2020 at 5:54 PM Zach Riggle <zachriggle@gmail.com> wrote:
Show 33 quoted lines
>
> Awesome, thanks!
>
>
> Zach Riggle
>
> On Wed, May 27, 2020 at 5:48 PM Jeff King <peff@peff.net> wrote:
> >
> > On Wed, May 27, 2020 at 05:29:08PM -0500, Zach Riggle wrote:
> >
> > > It looks like there is an issue with how the parser handles "goto"
> > > labels, as it treats them the same as a function name.
> >
> > By default, the function-finding isn't aware of the specific content in
> > the file. But you can associate extensions with particular types, like:
> >
> >   $ echo '*.cpp diff=cpp' >~/.gitattributes
> >   $ git config --global core.attributesFile ~/.gitattributes
> >   $ git grep --no-index --show-function -e FOO test2.cpp
> >   test2.cpp=int main() {
> >   test2.cpp:    FOO
> >   test2.cpp:    FOO
> >
> > Usually this is done in-repo, but since your example used --no-index, I
> > showed how to set up a per-user attribute file. The "diff" attribute
> > covers both diff and grep (for diff, the hunk headers will also show the
> > function).
> >
> > The "cpp" diff regexes are built-in to the git binary. We just don't
> > associate any filenames by default. You can also add your own; see the
> > section "Defining a custom hunk-header" from "git help attributes".
> >
> > -Peff
Zach Riggle· May 27, 2020, 23:05 UTC · re: Zach Riggle · lore

Re: git grep --show-function treats GOTO labels as function names

I made a mistake copy-pasting; things work correctly when the attribute is on the same line, but not when it is split:

$ cat attr.cpp
int main()
__attribute__ ( (no_sanitize("alignment")) )
{
    FOO
}
Zach Riggle
On Wed, May 27, 2020 at 6:04 PM Zach Riggle <zachriggle@gmail.com> wrote:
Show 50 quoted lines
>
> It looks like that does the trick for "goto" labels, but there are
> also some issue on function name parsing with attributes when they are
> split onto a second line.
>
> $ cat attr.cpp
> int main() __attribute__ ( (no_sanitize("alignment")) )
> {
>     FOO
> }
> $ git grep --no-index --show-function -e FOO attr.cpp
> attr.cpp=2=__attribute__ ( (no_sanitize("alignment")) )
> attr.cpp:4:    FOO
>
> Zach Riggle
>
> On Wed, May 27, 2020 at 5:54 PM Zach Riggle <zachriggle@gmail.com> wrote:
> >
> > Awesome, thanks!
> >
> >
> > Zach Riggle
> >
> > On Wed, May 27, 2020 at 5:48 PM Jeff King <peff@peff.net> wrote:
> > >
> > > On Wed, May 27, 2020 at 05:29:08PM -0500, Zach Riggle wrote:
> > >
> > > > It looks like there is an issue with how the parser handles "goto"
> > > > labels, as it treats them the same as a function name.
> > >
> > > By default, the function-finding isn't aware of the specific content in
> > > the file. But you can associate extensions with particular types, like:
> > >
> > >   $ echo '*.cpp diff=cpp' >~/.gitattributes
> > >   $ git config --global core.attributesFile ~/.gitattributes
> > >   $ git grep --no-index --show-function -e FOO test2.cpp
> > >   test2.cpp=int main() {
> > >   test2.cpp:    FOO
> > >   test2.cpp:    FOO
> > >
> > > Usually this is done in-repo, but since your example used --no-index, I
> > > showed how to set up a per-user attribute file. The "diff" attribute
> > > covers both diff and grep (for diff, the hunk headers will also show the
> > > function).
> > >
> > > The "cpp" diff regexes are built-in to the git binary. We just don't
> > > associate any filenames by default. You can also add your own; see the
> > > section "Defining a custom hunk-header" from "git help attributes".
> > >
> > > -Peff
Jeff King· May 27, 2020, 23:16 UTC · re: Zach Riggle · lore

Re: git grep --show-function treats GOTO labels as function names

On Wed, May 27, 2020 at 06:04:21PM -0500, Zach Riggle wrote:
Show 12 quoted lines
> It looks like that does the trick for "goto" labels, but there are
> also some issue on function name parsing with attributes when they are
> split onto a second line.
> 
> $ cat attr.cpp
> int main() __attribute__ ( (no_sanitize("alignment")) )
> {
>     FOO
> }
> $ git grep --no-index --show-function -e FOO attr.cpp
> attr.cpp=2=__attribute__ ( (no_sanitize("alignment")) )
> attr.cpp:4:    FOO

From your output, I assume the problematic input actually splits the attribute onto the second line?

I agree that's not ideal. The baked-in regex we use for matching C function lines is:

  $ git grep -nA4 cpp userdiff.c
  userdiff.c:173:PATTERNS("cpp",
  userdiff.c-174-  /* Jump targets or access declarations */
  userdiff.c-175-  "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])\n"
  userdiff.c-176-  /* functions/methods, variables, and compounds at top level */
  userdiff.c-177-  "^((::[[:space:]]*)?[A-Za-z_].*)$",

so we mistake it for a function name. I'm not sure how easy it is to do better, though. We can add a line like:

diff --git a/userdiff.c b/userdiff.c
index 1df884ef0b..de8e1a3d72 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -173,6 +173,8 @@ PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
 PATTERNS("cpp",
 	 /* Jump targets or access declarations */
 	 "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])\n"
+	 /* skip over attribute declarations are on their own lines */
+	 "!((::[[:space:]]*))?__attribute__\n"
 	 /* functions/methods, variables, and compounds at top level */
 	 "^((::[[:space:]]*)?[A-Za-z_].*)$",
 	 /* -- */

which works for your case, but would regress:

  __attribute__((whatever) int main()
  {
  FOO
  }

Handling both means skipping past the attribute, not counting it as a
function, and then checking for a plausible function afterwards. That's
a much trickier regex. But if you come up with something that works, I
think we'd be happy to take a patch. :)

You can also just override this regex via config for your personal use.
If you know you'd never use that style, then the problem becomes much
easier.

-Peff
Johannes Sixt· May 28, 2020, 19:01 UTC · re: Jeff King · lore

Re: git grep --show-function treats GOTO labels as function names

Am 28.05.20 um 01:16 schrieb Jeff King:
Show 29 quoted lines
> On Wed, May 27, 2020 at 06:04:21PM -0500, Zach Riggle wrote:
>> It looks like that does the trick for "goto" labels, but there are
>> also some issue on function name parsing with attributes when they are
>> split onto a second line.
>>
>> $ cat attr.cpp
>> int main() __attribute__ ( (no_sanitize("alignment")) )
>> {
>>     FOO
>> }
>> $ git grep --no-index --show-function -e FOO attr.cpp
>> attr.cpp=2=__attribute__ ( (no_sanitize("alignment")) )
>> attr.cpp:4:    FOO
> 
> From your output, I assume the problematic input actually splits the
> attribute onto the second line?
> 
> I agree that's not ideal. The baked-in regex we use for matching C
> function lines is:
> 
>   $ git grep -nA4 cpp userdiff.c
>   userdiff.c:173:PATTERNS("cpp",
>   userdiff.c-174-  /* Jump targets or access declarations */
>   userdiff.c-175-  "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])\n"
>   userdiff.c-176-  /* functions/methods, variables, and compounds at top level */
>   userdiff.c-177-  "^((::[[:space:]]*)?[A-Za-z_].*)$",
> 
> so we mistake it for a function name. I'm not sure how easy it is to do
> better, though. We can add a line like:

C and C++ have a very versatile syntax and it turned out to be virtually impossible to capture actual function definitions with a regex. See 8a2e8da367f7 ("userdiff: have 'cpp' hunk header pattern catch more C++ anchor points", 2014-03-21) for details.

With the current pattern, we catch probably 95% of the coding patterns and coding styles. The style under discussion falls into the remaining 5% because continuation lines are not indented.

-- Hannes

← back to recent threads