{"thread":{"id":"53567","subject":"git grep --show-function treats GOTO labels as function names","startedAt":"2020-05-27T22:29:22Z","lastAt":"2020-05-28T19:01:40Z","messageCount":7,"participants":["Zach Riggle","Jeff King","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"398702","messageId":"CAMP9c5k=Ci8eQyOdzW7a-sssgp9g9TJ+rxAAPF3YLDCr7t6wrg@mail.gmail.com","threadId":"53567","inReplyTo":null,"subject":"git grep --show-function treats GOTO labels as function names","fromName":"Zach Riggle","fromEmail":"zachriggle@gmail.com","sentAt":"2020-05-27T22:29:08Z","receivedAt":"2020-05-27T22:29:22Z","isPatch":false,"sender":{"key":"zachriggle@gmail.com","avatar":"https://gravatar.com/avatar/3eb62c0b142900b8c316fa0dcbf6f84079eb83e0d6fd6db79b44d6034c147604?d=mp&s=160"},"body":"Git grep is an amazing tool, and --show-function makes it very easy to\ncreate reports that contain not only line information, but the\ncontaining function as well.\n\nIt looks like there is an issue with how the parser handles \"goto\"\nlabels, as it treats them the same as a function name.\n\nIn the example below, there is only one function, with two matches,\nbut using --show-function indicates there are two function names.\n\n$ cat test2.cpp\nint main() {\n    FOO\nexit:\n    FOO\n}\n\n$ git --version\ngit version 2.26.2\n\n$ git grep --no-index --show-function -e FOO test2.cpp\ntest2.cpp=1=int main() {\ntest2.cpp:2:    FOO\ntest2.cpp=3=exit:\ntest2.cpp:4:    FOO\n\n\nZach Riggle\n"},{"id":"398704","messageId":"20200527224824.GA546534@coredump.intra.peff.net","threadId":"53567","inReplyTo":"CAMP9c5k=Ci8eQyOdzW7a-sssgp9g9TJ+rxAAPF3YLDCr7t6wrg@mail.gmail.com","subject":"Re: git grep --show-function treats GOTO labels as function names","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-27T22:48:24Z","receivedAt":"2020-05-27T22:48:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 27, 2020 at 05:29:08PM -0500, Zach Riggle wrote:\n\n> It looks like there is an issue with how the parser handles \"goto\"\n> labels, as it treats them the same as a function name.\n\nBy default, the function-finding isn't aware of the specific content in\nthe file. But you can associate extensions with particular types, like:\n\n  $ echo '*.cpp diff=cpp' >~/.gitattributes\n  $ git config --global core.attributesFile ~/.gitattributes\n  $ git grep --no-index --show-function -e FOO test2.cpp\n  test2.cpp=int main() {\n  test2.cpp:    FOO\n  test2.cpp:    FOO\n\nUsually this is done in-repo, but since your example used --no-index, I\nshowed how to set up a per-user attribute file. The \"diff\" attribute\ncovers both diff and grep (for diff, the hunk headers will also show the\nfunction).\n\nThe \"cpp\" diff regexes are built-in to the git binary. We just don't\nassociate any filenames by default. You can also add your own; see the\nsection \"Defining a custom hunk-header\" from \"git help attributes\".\n\n-Peff\n"},{"id":"398705","messageId":"CAMP9c5=kRAfKqfWL4AJg1m9c-3OwG1Vv=vBaiOhsD6abjtXH=A@mail.gmail.com","threadId":"53567","inReplyTo":"20200527224824.GA546534@coredump.intra.peff.net","subject":"Re: git grep --show-function treats GOTO labels as function names","fromName":"Zach Riggle","fromEmail":"zachriggle@gmail.com","sentAt":"2020-05-27T22:54:34Z","receivedAt":"2020-05-27T22:54:49Z","isPatch":false,"sender":{"key":"zachriggle@gmail.com","avatar":"https://gravatar.com/avatar/3eb62c0b142900b8c316fa0dcbf6f84079eb83e0d6fd6db79b44d6034c147604?d=mp&s=160"},"body":"Awesome, thanks!\n\n\nZach Riggle\n\nOn Wed, May 27, 2020 at 5:48 PM Jeff King <peff@peff.net> wrote:\n>\n> On Wed, May 27, 2020 at 05:29:08PM -0500, Zach Riggle wrote:\n>\n> > It looks like there is an issue with how the parser handles \"goto\"\n> > labels, as it treats them the same as a function name.\n>\n> By default, the function-finding isn't aware of the specific content in\n> the file. But you can associate extensions with particular types, like:\n>\n>   $ echo '*.cpp diff=cpp' >~/.gitattributes\n>   $ git config --global core.attributesFile ~/.gitattributes\n>   $ git grep --no-index --show-function -e FOO test2.cpp\n>   test2.cpp=int main() {\n>   test2.cpp:    FOO\n>   test2.cpp:    FOO\n>\n> Usually this is done in-repo, but since your example used --no-index, I\n> showed how to set up a per-user attribute file. The \"diff\" attribute\n> covers both diff and grep (for diff, the hunk headers will also show the\n> function).\n>\n> The \"cpp\" diff regexes are built-in to the git binary. We just don't\n> associate any filenames by default. You can also add your own; see the\n> section \"Defining a custom hunk-header\" from \"git help attributes\".\n>\n> -Peff\n"},{"id":"398707","messageId":"CAMP9c5m65hBXKgP76iUCGe79c_s5p106K6iwzJyPmm7fCsc7LA@mail.gmail.com","threadId":"53567","inReplyTo":"CAMP9c5=kRAfKqfWL4AJg1m9c-3OwG1Vv=vBaiOhsD6abjtXH=A@mail.gmail.com","subject":"Re: git grep --show-function treats GOTO labels as function names","fromName":"Zach Riggle","fromEmail":"zachriggle@gmail.com","sentAt":"2020-05-27T23:04:21Z","receivedAt":"2020-05-27T23:04:35Z","isPatch":false,"sender":{"key":"zachriggle@gmail.com","avatar":"https://gravatar.com/avatar/3eb62c0b142900b8c316fa0dcbf6f84079eb83e0d6fd6db79b44d6034c147604?d=mp&s=160"},"body":"It looks like that does the trick for \"goto\" labels, but there are\nalso some issue on function name parsing with attributes when they are\nsplit onto a second line.\n\n$ cat attr.cpp\nint main() __attribute__ ( (no_sanitize(\"alignment\")) )\n{\n    FOO\n}\n$ git grep --no-index --show-function -e FOO attr.cpp\nattr.cpp=2=__attribute__ ( (no_sanitize(\"alignment\")) )\nattr.cpp:4:    FOO\n\nZach Riggle\n\nOn Wed, May 27, 2020 at 5:54 PM Zach Riggle <zachriggle@gmail.com> wrote:\n>\n> Awesome, thanks!\n>\n>\n> Zach Riggle\n>\n> On Wed, May 27, 2020 at 5:48 PM Jeff King <peff@peff.net> wrote:\n> >\n> > On Wed, May 27, 2020 at 05:29:08PM -0500, Zach Riggle wrote:\n> >\n> > > It looks like there is an issue with how the parser handles \"goto\"\n> > > labels, as it treats them the same as a function name.\n> >\n> > By default, the function-finding isn't aware of the specific content in\n> > the file. But you can associate extensions with particular types, like:\n> >\n> >   $ echo '*.cpp diff=cpp' >~/.gitattributes\n> >   $ git config --global core.attributesFile ~/.gitattributes\n> >   $ git grep --no-index --show-function -e FOO test2.cpp\n> >   test2.cpp=int main() {\n> >   test2.cpp:    FOO\n> >   test2.cpp:    FOO\n> >\n> > Usually this is done in-repo, but since your example used --no-index, I\n> > showed how to set up a per-user attribute file. The \"diff\" attribute\n> > covers both diff and grep (for diff, the hunk headers will also show the\n> > function).\n> >\n> > The \"cpp\" diff regexes are built-in to the git binary. We just don't\n> > associate any filenames by default. You can also add your own; see the\n> > section \"Defining a custom hunk-header\" from \"git help attributes\".\n> >\n> > -Peff\n"},{"id":"398708","messageId":"CAMP9c5nROh5DxbfwgKL3vNWsr=RYYgkEHCeaEMVN3gq1xkPkLA@mail.gmail.com","threadId":"53567","inReplyTo":"CAMP9c5m65hBXKgP76iUCGe79c_s5p106K6iwzJyPmm7fCsc7LA@mail.gmail.com","subject":"Re: git grep --show-function treats GOTO labels as function names","fromName":"Zach Riggle","fromEmail":"zachriggle@gmail.com","sentAt":"2020-05-27T23:05:14Z","receivedAt":"2020-05-27T23:05:28Z","isPatch":false,"sender":{"key":"zachriggle@gmail.com","avatar":"https://gravatar.com/avatar/3eb62c0b142900b8c316fa0dcbf6f84079eb83e0d6fd6db79b44d6034c147604?d=mp&s=160"},"body":"I made a mistake copy-pasting; things work correctly when the\nattribute is on the same line, but not when it is split:\n\n$ cat attr.cpp\nint main()\n__attribute__ ( (no_sanitize(\"alignment\")) )\n{\n    FOO\n}\n\nZach Riggle\n\nOn Wed, May 27, 2020 at 6:04 PM Zach Riggle <zachriggle@gmail.com> wrote:\n>\n> It looks like that does the trick for \"goto\" labels, but there are\n> also some issue on function name parsing with attributes when they are\n> split onto a second line.\n>\n> $ cat attr.cpp\n> int main() __attribute__ ( (no_sanitize(\"alignment\")) )\n> {\n>     FOO\n> }\n> $ git grep --no-index --show-function -e FOO attr.cpp\n> attr.cpp=2=__attribute__ ( (no_sanitize(\"alignment\")) )\n> attr.cpp:4:    FOO\n>\n> Zach Riggle\n>\n> On Wed, May 27, 2020 at 5:54 PM Zach Riggle <zachriggle@gmail.com> wrote:\n> >\n> > Awesome, thanks!\n> >\n> >\n> > Zach Riggle\n> >\n> > On Wed, May 27, 2020 at 5:48 PM Jeff King <peff@peff.net> wrote:\n> > >\n> > > On Wed, May 27, 2020 at 05:29:08PM -0500, Zach Riggle wrote:\n> > >\n> > > > It looks like there is an issue with how the parser handles \"goto\"\n> > > > labels, as it treats them the same as a function name.\n> > >\n> > > By default, the function-finding isn't aware of the specific content in\n> > > the file. But you can associate extensions with particular types, like:\n> > >\n> > >   $ echo '*.cpp diff=cpp' >~/.gitattributes\n> > >   $ git config --global core.attributesFile ~/.gitattributes\n> > >   $ git grep --no-index --show-function -e FOO test2.cpp\n> > >   test2.cpp=int main() {\n> > >   test2.cpp:    FOO\n> > >   test2.cpp:    FOO\n> > >\n> > > Usually this is done in-repo, but since your example used --no-index, I\n> > > showed how to set up a per-user attribute file. The \"diff\" attribute\n> > > covers both diff and grep (for diff, the hunk headers will also show the\n> > > function).\n> > >\n> > > The \"cpp\" diff regexes are built-in to the git binary. We just don't\n> > > associate any filenames by default. You can also add your own; see the\n> > > section \"Defining a custom hunk-header\" from \"git help attributes\".\n> > >\n> > > -Peff\n"},{"id":"398709","messageId":"20200527231628.GC546534@coredump.intra.peff.net","threadId":"53567","inReplyTo":"CAMP9c5m65hBXKgP76iUCGe79c_s5p106K6iwzJyPmm7fCsc7LA@mail.gmail.com","subject":"Re: git grep --show-function treats GOTO labels as function names","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-27T23:16:28Z","receivedAt":"2020-05-27T23:16:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 27, 2020 at 06:04:21PM -0500, Zach Riggle wrote:\n\n> It looks like that does the trick for \"goto\" labels, but there are\n> also some issue on function name parsing with attributes when they are\n> split onto a second line.\n> \n> $ cat attr.cpp\n> int main() __attribute__ ( (no_sanitize(\"alignment\")) )\n> {\n>     FOO\n> }\n> $ git grep --no-index --show-function -e FOO attr.cpp\n> attr.cpp=2=__attribute__ ( (no_sanitize(\"alignment\")) )\n> attr.cpp:4:    FOO\n\nFrom your output, I assume the problematic input actually splits the\nattribute onto the second line?\n\nI agree that's not ideal. The baked-in regex we use for matching C\nfunction lines is:\n\n  $ git grep -nA4 cpp userdiff.c\n  userdiff.c:173:PATTERNS(\"cpp\",\n  userdiff.c-174-  /* Jump targets or access declarations */\n  userdiff.c-175-  \"!^[ \\t]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])\\n\"\n  userdiff.c-176-  /* functions/methods, variables, and compounds at top level */\n  userdiff.c-177-  \"^((::[[:space:]]*)?[A-Za-z_].*)$\",\n\nso we mistake it for a function name. I'm not sure how easy it is to do\nbetter, though. We can add a line like:\n\ndiff --git a/userdiff.c b/userdiff.c\nindex 1df884ef0b..de8e1a3d72 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -173,6 +173,8 @@ PATTERNS(\"tex\", \"^(\\\\\\\\((sub)*section|chapter|part)\\\\*{0,1}\\\\{.*)$\",\n PATTERNS(\"cpp\",\n \t /* Jump targets or access declarations */\n \t \"!^[ \\t]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])\\n\"\n+\t /* skip over attribute declarations are on their own lines */\n+\t \"!((::[[:space:]]*))?__attribute__\\n\"\n \t /* functions/methods, variables, and compounds at top level */\n \t \"^((::[[:space:]]*)?[A-Za-z_].*)$\",\n \t /* -- */\n\nwhich works for your case, but would regress:\n\n  __attribute__((whatever) int main()\n  {\n  FOO\n  }\n\nHandling both means skipping past the attribute, not counting it as a\nfunction, and then checking for a plausible function afterwards. That's\na much trickier regex. But if you come up with something that works, I\nthink we'd be happy to take a patch. :)\n\nYou can also just override this regex via config for your personal use.\nIf you know you'd never use that style, then the problem becomes much\neasier.\n\n-Peff\n"},{"id":"398775","messageId":"07f56d7a-45cc-2d28-ec48-51b95015550e@kdbg.org","threadId":"53567","inReplyTo":"20200527231628.GC546534@coredump.intra.peff.net","subject":"Re: git grep --show-function treats GOTO labels as function names","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2020-05-28T19:01:25Z","receivedAt":"2020-05-28T19:01:40Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.05.20 um 01:16 schrieb Jeff King:\n> On Wed, May 27, 2020 at 06:04:21PM -0500, Zach Riggle wrote:\n>> It looks like that does the trick for \"goto\" labels, but there are\n>> also some issue on function name parsing with attributes when they are\n>> split onto a second line.\n>>\n>> $ cat attr.cpp\n>> int main() __attribute__ ( (no_sanitize(\"alignment\")) )\n>> {\n>>     FOO\n>> }\n>> $ git grep --no-index --show-function -e FOO attr.cpp\n>> attr.cpp=2=__attribute__ ( (no_sanitize(\"alignment\")) )\n>> attr.cpp:4:    FOO\n> \n> From your output, I assume the problematic input actually splits the\n> attribute onto the second line?\n> \n> I agree that's not ideal. The baked-in regex we use for matching C\n> function lines is:\n> \n>   $ git grep -nA4 cpp userdiff.c\n>   userdiff.c:173:PATTERNS(\"cpp\",\n>   userdiff.c-174-  /* Jump targets or access declarations */\n>   userdiff.c-175-  \"!^[ \\t]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])\\n\"\n>   userdiff.c-176-  /* functions/methods, variables, and compounds at top level */\n>   userdiff.c-177-  \"^((::[[:space:]]*)?[A-Za-z_].*)$\",\n> \n> so we mistake it for a function name. I'm not sure how easy it is to do\n> better, though. We can add a line like:\n\nC and C++ have a very versatile syntax and it turned out to be virtually\nimpossible to capture actual function definitions with a regex. See\n8a2e8da367f7 (\"userdiff: have 'cpp' hunk header pattern catch more C++\nanchor points\", 2014-03-21) for details.\n\nWith the current pattern, we catch probably 95% of the coding patterns\nand coding styles. The style under discussion falls into the remaining\n5% because continuation lines are not indented.\n\n-- Hannes\n"}]}