Skip to content
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

No need to pass CheckedScopeSpecifier around in SemaBounds.cpp #1151

Open
mgrang opened this issue Aug 10, 2021 · 3 comments
Open

No need to pass CheckedScopeSpecifier around in SemaBounds.cpp #1151

mgrang opened this issue Aug 10, 2021 · 3 comments
Labels
priority:3 This labels bugs that are not very critical but still need to be addressed. work item This labels issues that are not exactly bugs but are about improvements.

Comments

@mgrang
Copy link

mgrang commented Aug 10, 2021

After #1142 we can get the CheckedScopeSpecifier (CSS) for every statement by invoking S->getCheckedScopeSpecifier().

In SemaBounds.cpp we pass CSS around to functions. This should no longer be needed.

@mgrang mgrang added priority:3 This labels bugs that are not very critical but still need to be addressed. work item This labels issues that are not exactly bugs but are about improvements. labels Aug 10, 2021
@kkjeer
Copy link
Contributor

kkjeer commented Aug 14, 2021

Will this work for all subexpressions in a statement?

For example, if we have:

void f(_Array_ptr<int> p : count(1), _Array_ptr<int> q : count(2)) {
  _Checked {
    p = q, p++;
  }
}

At the assignment p = q (subexpression of the statement p = q, p++, we call CheckBinaryOperator(e, ..., CSS), where e is the binary operator expression p = q. Can we tell that p = q is in a checked scope without passing the CSS argument to CheckBinaryOperator?

@mgrang
Copy link
Author

mgrang commented Aug 16, 2021

Checked Scope Specifiers (CSS) are set per statement. Sub-expressions do not have CSS. So p = q, p++ would have a CSS, not the subexpression p = q. The CSS for a subexpression should be the same as its containing statement. So I guess you could determine which statement a nested sub-expression belongs to and use its CSS.

@sulekhark
Copy link
Contributor

This is a good reason to reconsider the following (review comment in PR #1142):
The field CSS is added as a sibling field of sClass in the class StmtBitfields - i.e. it is present for every "Stmt". The field sClass is initialized in the constructor for Stmt at line 1200 (in AST/Stmt.h on master branch). It is also possible to initialize the field CSS in the same constructor because GetCheckedScopeInfo (which reads CheckingKind that stores the currently applicable value of the checked scope specification) is a Sema function. This will avoid looping over the statements in the body of a compound statement, which may be unavoidable if the CSS field is set in ActOnCompoundStmt.

It may not be a good idea to have to figure out the statement which contains the subexpression for which the CSS value is required, each time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:3 This labels bugs that are not very critical but still need to be addressed. work item This labels issues that are not exactly bugs but are about improvements.
Projects
None yet
Development

No branches or pull requests

3 participants