Skip to content

fix crash with templates containing shifts #6

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
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion c.c
Original file line number Diff line number Diff line change
Expand Up @@ -1388,10 +1388,16 @@ static void skipToMatch (const char *const pair)
{
if (CollectingSignature)
vStringPut (Signature, c);


if (c == begin)
{
char s = cppGetc();
if (s == '<' && s == c) {
// templates can still contain "<<" ot "<<="
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't breaking out of here put the parsing in a weird state, as now it will go forward parsing while inside a template parameter list?

(Also completely missed that there was already a pull request for this when one of my users pinged me about this being a problem w/FastLED).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really understand what you mean, this part of the patch looks for "<<" pattern inside a template, so going on parsing while ignoring the shift should be safe.
The other part takes care of not failing completely if we are unable to find the closing '>'

Whit this patch the snippet

template <int P> class c {};
c< 8 > bVar;
c< 1<<8 > aVar;
c< 1>>8 > cVar;

produces

bVar    /home/martino/arduino-builder/src/arduino.cc/builder/test/sketch11/sketch_fastleds.ino  /^c< 8 > bVar;$/;"  kind:variable   line:30
aVar    /home/martino/arduino-builder/src/arduino.cc/builder/test/sketch11/sketch_fastleds.ino  /^c< 1<<8 > aVar;$/;"   kind:variable   line:31
cVar    /home/martino/arduino-builder/src/arduino.cc/builder/test/sketch11/sketch_fastleds.ino  /^c< 1>>8 > cVar;$/;"   kind:variable   line:32

Should be ok, right? 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's still cases that it will miss - for example:

template <int P> class c {};
c< 8 > bVar;
c< 1<<8 > aVar;

template<int X> func( c< 1<<X> & aParam) {
}

it will fail to parse out the definition for func, only dumping out aVar/bVar - because it's punted on trying to keep things balanced, it's just going to walk forward until the {.

The desired output would be:

./ctags -u --language-force=c++ -f - --c++-kinds=svpf --fields=KSTtzns --line-directives /tmp/x.cpp --verbose
bVar    /tmp/x.cpp  /^c< 8 > bVar;$/;"  kind:variable   line:2
aVar    /tmp/x.cpp  /^c< 1<<8 > aVar;$/;"   kind:variable   line:3
func    /tmp/x.cpp  /^template<int X> func( c< 1<<X> & aVar) {$/;"  kind:function   line:5  signature:( c< 1<<X> & aVar)    returntype:template

} else {
cppUngetc(s);
}
++matchLevel;
if (braceFormatting && getDirectiveNestLevel () != initialLevel)
{
Expand All @@ -1408,6 +1414,16 @@ static void skipToMatch (const char *const pair)
break;
}
}
/* early out if matching "<>" and we encounter a ";" or "{" to mitigate
* match problems with C++ generics containing a static expression like
* foo<X<Y> bar;
* normally neither ";" nor "{" could appear inside "<>" anyway. */
else if (isLanguage (Lang_cpp) && begin == '<' &&
(c == ';' || c == '{'))
{
cppUngetc (c);
break;
}
}
if (c == EOF)
{
Expand Down