-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
C++: Improve parameter naming #17778
Conversation
result = this.getAnEffectiveDeclarationEntry() and result.getName() != "" | ||
result = this.getAnEffectiveDeclarationEntry() and | ||
exists(string name | var_decls(unresolveElement(result), _, _, name, _) | name != "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is needed to avoid non-monotonic recursion after extending VariableDeclarationEntry.getName()
. It does not affect the ouput.
exists(string idx | | ||
idx = | ||
((this.getIndex() + 1).toString() + "th") | ||
.replaceAll("1th", "1st") | ||
.replaceAll("2th", "2nd") | ||
.replaceAll("3th", "3rd") | ||
.replaceAll("11st", "11th") | ||
.replaceAll("12nd", "12th") | ||
.replaceAll("13rd", "13th") and | ||
exists(string anon | | ||
anon = "(unnamed parameter " + this.getIndex().toString() + ")" and | ||
if exists(this.getCanonicalName()) | ||
then | ||
result = "declaration of " + this.getCanonicalName() + " as anonymous " + idx + " parameter" | ||
else result = "declaration of " + idx + " parameter" | ||
then result = "declaration of " + this.getCanonicalName() + " as " + anon | ||
else result = "declaration of " + anon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This make the string representation of parameter declaration entries consistent with the representation of parameters, which it wasn't.
or | ||
name = "" and | ||
not this instanceof ParameterDeclarationEntry and | ||
result = this.getVariable().(Parameter).getName() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the missing string representation I found while working on requires expressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the internal PR associated with #17740
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I see you've already done a DCA run and it looks clean to me.
.replaceAll("3th", "3rd") | ||
.replaceAll("11st", "11th") | ||
.replaceAll("12nd", "12th") | ||
.replaceAll("13rd", "13th") and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will confess to being a bit sad that we lose this wonderful, wonky logic.
See inline comments for explanation.
Pull Request checklist
All query authors
.qhelp
. See the documentation in this repository.Internal query authors only
.ql
,.qll
, or.qhelp
files. See the documentation (internal access required).