-
Notifications
You must be signed in to change notification settings - Fork 1.7k
CPP: QLDoc enhancements #2309
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
CPP: QLDoc enhancements #2309
Conversation
… figure out how to describe unknown and erroneous types.)
…erations and built-in operations.
…sorted complex number operations, where surface syntax could not be determined.
assorted complex/imaginary arithmetic operations.
…builtin_offsetof.
similar to array initializers.
*/ | ||
abstract class Assignment extends Operation { | ||
/** Gets the lvalue of this assignment. */ | ||
/** Gets the *lvalue* of this assignment. */ |
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.
Can anyone remember whether we decided to keep this formatting (*lvalue*
)?
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.
We decided against formatting code with asterisks, but I think in this case it's meant to denote italics because lvalue is not an English word. Allows writing italics with single asterisks (bold is double asterisks), but it's more consistent if we stick to underscores, changing this to _lvalue_
.
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've done a search-and-replace to change *lvalue*
into _lvalue_
(and similarly for *rvalue*
etc).
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'm not done -- I'll continue reviewing later.
I should be clear that none of my comments so far are blocking for this PR. |
@@ -7,6 +7,9 @@ abstract class UnaryLogicalOperation extends UnaryOperation { } | |||
|
|||
/** | |||
* A C/C++ logical not expression. |
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.
* A C/C++ logical not expression. | |
* A C/C++ logical NOT expression. |
for consistency
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'm not sure I agree with this one. Do we capitalize 'not' anywhere else?
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.
The changes right below in the same file change "and" to "AND" and "or" to "OR". The same changes are in BitwiseOperation.qll
, although bitwise NOT is called complement there.
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.
Which would you prefer? I can make a quick follow-up PR to make this consistent either way.
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 prefer NOT because it's consistent with AND/OR and because everyone will understand it.
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
Co-Authored-By: Jonas Jensen <[email protected]>
All comments addressed. I'm on holiday tomorrow so ideally this wants to be merged this evening. I'll keep an eye on it. |
This is Zem's PR [CPP-418] QLDoc enhancements with some additional fixes: