Don't set init and init style of a declaration manually #1232
Conversation
This file contains 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
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| // Clang expects direct inits to be wrapped either in InitListExpr or | ||
| // ParenListExpr. | ||
| if (DirectInit && !isa<InitListExpr>(Init) && !isa<ParenListExpr>(Init)) |
There was a problem hiding this comment.
warning: no header providing "clang::isa" is directly included [misc-include-cleaner]
if (DirectInit && !isa<InitListExpr>(Init) && !isa<ParenListExpr>(Init))
^
PetroZarytskyi
force-pushed
the
init-style
branch
from
f2919dd to
dcfbee8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1232 +/- ##
==========================================
+ Coverage 94.64% 94.66% +0.02%
==========================================
Files 51 51
Lines 8890 8886 -4
==========================================
- Hits 8414 8412 -2
+ Misses 476 474 -2
|
PetroZarytskyi
force-pushed
the
init-style
branch
3 times, most recently
from
837b88d to
3b6762b
Compare
vgvassilev
reviewed
Feb 3, 2025
| // not reset the init and we have to do it manually. | ||
| VD->setInit(nullptr); | ||
| m_Sema.ActOnUninitializedDecl(VD); | ||
| return; |
There was a problem hiding this comment.
Does ActOnUninitializedDecl set the InitStyle, too? If so we should update the comment.
There was a problem hiding this comment.
Does init style matter when it comes to uninitialized decls?
There was a problem hiding this comment.
Depends on what clang does... Probably does not but then we need to update the documentation comment of SetDeclInit.
PetroZarytskyi
force-pushed
the
init-style
branch
2 times, most recently
from
4d8732f to
4dabcd7
Compare
vgvassilev
force-pushed
the
init-style
branch
from
4dabcd7 to
c373885
Compare
vgvassilev
reviewed
Feb 4, 2025
| @@ -16,6 +16,8 @@ | |||
| #include "clang/Sema/ParsedAttr.h" | |||
| #include "clang/Sema/Sema.h" | |||
| #include <array> | |||
| #include <clang/AST/Type.h> | |||
| #include <llvm/ADT/StringRef.h> | |||
There was a problem hiding this comment.
We should put these before array and sort alphabetically.
There was a problem hiding this comment.
I tried ordering everything alphabetically but clang-format reverted all changes. I separated the includes into different blocks, clang-format seems to be okay with this. I hope this is better.
| @@ -5,6 +5,7 @@ | |||
|
|
|||
| #include "clang/AST/TemplateName.h" | |||
| #include "clang/Sema/Lookup.h" | |||
| #include <clang/AST/Decl.h> | |||
PetroZarytskyi
force-pushed
the
init-style
branch
2 times, most recently
from
4f17027 to
9415eca
Compare
Initialization styles should not be set manually, instead, they should be deduced by clang based on the types of the declaration and init, as well as on whether the declaration is direct. e.g. direct initialization of `std::vector vec` with `other` should look like ``` std::vector vec(other); ``` and not ``` std::vector vec = other; ```
Sometimes we initialize declarations using ``Decl::setInit``, which is a low-level function used by clang internally. This commit introduces a wrapper of ``Sema::AddInitializerToDecl`` called ``SetDeclInit``.
PetroZarytskyi
force-pushed
the
init-style
branch
from
9415eca to
cf8540b
Compare
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.
This PR addresses 2 issues:
e.g. direct initialization of
std::vector vecwithothershould look likeand not
Decl::setInit, which is a low-level function used by clang internally. This PR introduces a wrapper ofSema::AddInitializerToDeclcalledSetDeclInit.