Add relative alignment to StmtProfiler::VisitRangeBoundsExpr #692
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sunnychatterjee
approved these changes
Oct 4, 2019
lib/AST/StmtProfile.cpp
Outdated
|
|
||
| if (S->hasRelativeBoundsClause()) { | ||
| RelativeBoundsClause *R = S->getRelativeBoundsClause(); | ||
| switch (R->getClauseKind()) { |
There was a problem hiding this comment.
Nit: There are only two valid clause kinds for RelativeBoundsClause. An if-statement might be simpler to use here rather than a switch-case. For example, see ASTDumper::VisitRangeBoundsExpr.
lib/AST/StmtProfile.cpp
Outdated
| case RelativeBoundsClause::Kind::Const: | ||
| VisitExpr(cast<RelativeConstExprBoundsClause>(R)->getConstExpr()); | ||
| break; | ||
| default: { |
There was a problem hiding this comment.
You can add this here: llvm_unreachable("unexpected kind field of relative bounds clause");
| // Test declaration and definition of functions with a range bounds parameter with a rel_align_value clause. | ||
| // | ||
|
|
||
| const int val1 = 6; |
There was a problem hiding this comment.
Have you tried with more complicated compile time expressions like constexpr int val3 = foo(); where "foo" returns '5+1' as a constexpr?
There was a problem hiding this comment.
This test is for C, not C++, so we don't have constexpr available.
dtarditi
reviewed
Oct 4, 2019
| // Tests for checking that function declarations involving range bounds | ||
| // account for relative alignment information. | ||
| // | ||
| // RUN: %clang -cc1 -fcheckedc-extension -Wcheck-bounds-decls -verify %s |
There was a problem hiding this comment.
You don't need the -fcheckedc-extension flag.
| // Test declaration and definition of functions with a range bounds parameter with a rel_align_value clause. | ||
| // | ||
|
|
||
| const int val1 = 6; |
There was a problem hiding this comment.
This test is for C, not C++, so we don't have constexpr available.
lib/AST/StmtProfile.cpp
Outdated
| case RelativeBoundsClause::Kind::Const: | ||
| VisitExpr(cast<RelativeConstExprBoundsClause>(R)->getConstExpr()); | ||
| break; | ||
| default: { |
mgrang
reviewed
Oct 8, 2019
lib/AST/StmtProfile.cpp
Outdated
| if (S->hasRelativeBoundsClause()) { | ||
| RelativeBoundsClause *R = S->getRelativeBoundsClause(); | ||
| RelativeBoundsClause::Kind CK = R->getClauseKind(); | ||
| if (CK == RelativeBoundsClause::Kind::Type) { |
There was a problem hiding this comment.
You can leave out braces in single statement if's.
mgrang
pushed a commit
that referenced
this pull request
Oct 11, 2019
Cherry-picked from commit da2b542 Author: Katherine Kjeer <[email protected]> Commit: GitHub <[email protected]> Add relative alignment to StmtProfiler::VisitRangeBoundsExpr (#692) * Remove unused BoundsCastExpr::SyntaxType enum * Visit relative bounds clause in StmtProfiler::VisitRangeBoundsExpr * Add nullptr to ID for RangeBoundsExpr with no relative alignment * Add tests for range bounds with relative alignment * Use if/else instead of switch * Remove unnecessary -fcheckedc-extension from test * Remove braces from single statement if's
Machiry
pushed a commit
to Machiry/checkedc-clang
that referenced
this pull request
Jan 21, 2022
We previously used the _3COptions structure to store command line options immediately after they were parsed, but later copied the option values out of this structure and into global variables for use in the rest of the program. This change deletes the global variables and replaces them with a single global instance of the _3COptions structure.
Machiry
pushed a commit
to Machiry/checkedc-clang
that referenced
this pull request
Jan 21, 2022
This is a follow up PR from checkedc#692 that moves all command line options into the _3COptions structure, moves all of 3C's command line options into the same category so that all options are shown by -help (fixes checkedc#393), and reformats some of the code around creating command line options. It also removes the the option -enable-itypeprop which was not referenced anywhere in the code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Visit the relative bounds clause (if any) for a RangeBoundsExpr in StmtProfiler::VisitRangeBoundsExpr. This means that examples such as the following now do not compile:
This addresses the first part of issue #331 (VisitRangeBoundsExpr needs to examine relative alignment information). Issue #691 tracks work related to VisitBoundsCastExpr and will be addressed in a separate PR.
Tests: