Skip to content

[CPP-418] QLDoc enhancements #1864

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 20 commits into from

Conversation

zlaski-semmle
Copy link
Contributor

Type.qll for now.

@zlaski-semmle zlaski-semmle requested a review from a team as a code owner September 3, 2019 19:14
@zlaski-semmle zlaski-semmle changed the title [CPP-418] QLDoc enhancement. [CPP-418] QLDoc enhancements Sep 3, 2019
@jbj jbj added the C++ label Sep 4, 2019
@zlaski-semmle
Copy link
Contributor Author

Which modules still need fixing (and are not being worked on by someone else)?

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Some comments.

Thank you for taking on this task!

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Comments on recent changes.

* `getInitializingExpr()` is `x < y`.
* A C++ variable declaration inside the conditional expression of a `while` or `if`
* compound statement. Declaring a variable this way narrows its lifetime and scope
* to be strictly the compound statement itself. For example:
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume they can occur in a for loop as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, they cannot. The initializer clause of a for can indeed contain declaration(s), but they are not conditional ones (i.e., compared against zero) as in while or if.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about:

for (i = 0; bool c = x < y; x++) ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that is not legal (though an interesting idea); the second clause must be an expression, not a declaration.

Copy link
Contributor

Choose a reason for hiding this comment

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

It works for me: #1945

@@ -997,7 +1055,7 @@ private Expr getStmtResultExpr(Stmt stmt) {
}

/**
* A C/C++ this expression.
* The C++ `this` pointer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it better to describe this as a pointer rather than an expression?

(I agree with replacing "C/C++" with just "C++" here).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling it a pointer is just simpler (and the way most C++ programmers think about this).

@geoffw0 geoffw0 mentioned this pull request Nov 13, 2019
@geoffw0
Copy link
Contributor

geoffw0 commented Nov 13, 2019

Now #2309.

@geoffw0 geoffw0 closed this Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants