-
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++: Add more MaD models for ATL string classes #18261
Conversation
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.
Looks good, at least from what I can remember from 25 years ago. I noted one trivial missing model, not blocking.
- ["", "CSimpleStringT", True, "operator+=", "(wchar_t)", "", "Argument[-1]", "ReturnValue[*]", "taint", "manual"] | ||
- ["", "CSimpleStringT", True, "operator+=", "(wchar_t)", "", "Argument[0]", "ReturnValue[*]", "taint", "manual"] | ||
- ["", "CSimpleStringT", True, "operator+=", "(wchar_t)", "", "Argument[0]", "Argument[-1]", "taint", "manual"] | ||
- ["", "CSimpleStringT", True, "operator=", "", "", "Argument[*0]", "Argument[-1]", "value", "manual"] |
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.
Does operator=
return a CSimpleStringT&
? If so, Argument[*0]
should propagate to ReturnValue[*]
as well.
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.
Indeed it does: https://learn.microsoft.com/en-us/cpp/atl-mfc-shared/reference/csimplestringt-class?view=msvc-170#operator_eq. Turns out I actually forgot that flow in a bunch of other models. I've included all of those in e777377. Thanks!
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.
Minor corrections / comments, otherwise LGTM. Thanks for addressing the missing operator=
return value models in addition to the main purpose of this PR.
Co-authored-by: Geoffrey White <[email protected]>
Yet another round of 🦅-eyed reviewing, Geoffry! Thanks a lot. I've responded to all the comments now 🙇 |
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.
Follow-up to #18136
Note that I also fixed another MaD bug: we didn't handle varargs. This has been fixed in c5bb907