Skip to content

Fix function rewriting with r-paren between prototype and definition#394

Merged
john-h-kastner merged 6 commits into
mainfrom
fix_392
Feb 8, 2021
Merged

Fix function rewriting with r-paren between prototype and definition#394
john-h-kastner merged 6 commits into
mainfrom
fix_392

Conversation

@john-h-kastner

Copy link
Copy Markdown
Collaborator

Fixes #392. A right parentheses appearing between a function prototype and the
beginning of its definition caused anything falling between the
parenthesis and the actual end of the prototype to be deleted. This
problem is fixed by using clang provided source locations instead of
locating the closing parenthesis by iterating of the source code
characters.

A right parentheses appearing between a function prototype and the
beginning of its definition caused anything falling between the
parenthesis and the actual end of the prototype to be deleted. This
problem is fixed by using clang provided source locations instead of
locating the closing parenthesis by iterating of the source code
characters.
Comment on lines -121 to -122
// Note: getFunctionDeclarationEnd is used instead of getRParenLoc so that
// itypes are deleted correctly when --remove-itypes is used.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is --remove-itypes still available? I assume we don't want it anymore, and the new code here doesn't use it, so if it lingers we could remove it here or in a later pull request.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Remove itypes still works on the 5c build, although we'll probably want to update it to include removing unnecessary casts. This comment is removed because the conditional below it handles overwriting the bounds and interop type expression correctly in the first two branches without needing to fall through to getFunctionDeclarationEnd

Comment thread clang/include/clang/3C/RewriteUtils.h Outdated
Comment on lines +131 to +132
if (!End.isValid())
End = getFunctionDeclarationEnd(Decl, SM);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is getFunctionDeclarationEnd used elsewhere? It's unfortunate that this condition needs to be here, since it was the offending code, but if it's a hack I think it's better inline rather than exposed to be reused.

@kyleheadley

Copy link
Copy Markdown
Member

looks good to me

@mwhicks1

mwhicks1 commented Feb 5, 2021

Copy link
Copy Markdown
Member

I this one ready to go? Seems that Kyle approved it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ifdef branch removed

3 participants