Skip to content

Clean up code in CheckBoundsDeclarations class.#498

Merged
dtarditi merged 2 commits into
masterfrom
issue497
Jun 12, 2018
Merged

Clean up code in CheckBoundsDeclarations class.#498
dtarditi merged 2 commits into
masterfrom
issue497

Conversation

@dtarditi

Copy link
Copy Markdown
Member
  • There's some code that appears to be unnecessary in VisitVarDecl. We should remove it. VisitCastExpr should already be taking care of this.
  • In addition, add comments describing the the difference between the Visit* and Traverse* methods. It's not immediately obvious what the difference is.
  • Rename TraverseVarDecl to TraverseTopLevelVarDecl for clarity.

Testing:

  • Passed local testing on Windows.
  • Passed automated testing on x64 Linux: debug clang and LNT tests.

- There's some code that appears to be unnecessary in VisitVarDecl. We should
remove it.  VisitCastExpr should already be taking care of this.
- In addition, describe the difference between the Visit* and Traverse* methods.
It's not immediately obvious whhat the difference is.
- Rename TraverseVarDecl to TraverseTopLevelVarDecl for clarity.
@dtarditi dtarditi requested a review from sxl463 June 11, 2018 22:38
Comment thread lib/Sema/SemaBounds.cpp Outdated
if (!D->isLocalVarDeclOrParm())
CheckBoundsDeclarations(*this, nullptr).TraverseVarDecl(D, getCurScope()->isCheckedScope());
if (!D->isLocalVarDeclOrParm()) {
auto Checker = CheckBoundsDeclarations(*this, nullptr);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to update the existed code right after this change? Just like this snippet:
void Sema::CheckTopLevelBoundsDecls(VarDecl *D) {
if (!D->isLocalVarDeclOrParm())
CheckBoundsDeclarations(*this, nullptr).TraverseVarDecl(D, getCurScope()->isCheckedScope());
}

@dtarditi dtarditi merged commit 809b250 into master Jun 12, 2018
@dtarditi

Copy link
Copy Markdown
Member Author

Thanks for the CR - I removed the use of auto and just used an initializer instead.

@dtarditi dtarditi deleted the issue497 branch June 13, 2018 17:06
sulekhark pushed a commit that referenced this pull request Jul 8, 2021
The code for deciding if a function should get an itype was duplicated
for function declarations and function pointer types. The function
pointer version of the code had a bug in it that caused issue #498.
The duplicated code has been extracted into a pair of functions that are
reused for functions and function pointers.

A lot of the lines changed in this PR are caused by an EnvironmentMap&
parameter being changed to Constraints&. This lets the function pointer
code call solutionEqualTo which is needed for correct itype insertion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants