[clang] missing changes to the Rewriter #152845
Merged
+37
−23
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
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis completes #147835 by adapting the changes in the API to the rewriter. Fixes build failures such as reported here: #147835 (comment) We previously missed this because it was not tested in pre-commit CI. This patch also addresses that problem. Full diff: https://github.com/llvm/llvm-project/pull/152845.diff 3 Files Affected:
diff --git a/.ci/monolithic-linux.sh b/.ci/monolithic-linux.sh
index 75729b3fd5f6a..6e5d3634c2a48 100755
--- a/.ci/monolithic-linux.sh
+++ b/.ci/monolithic-linux.sh
@@ -48,6 +48,7 @@ cmake -S "${MONOREPO_ROOT}"/llvm -B "${BUILD_DIR}" \
-D CMAKE_PREFIX_PATH="${HOME}/.local" \
-D CMAKE_BUILD_TYPE=Release \
-D CLANG_ENABLE_CIR=${enable_cir} \
+ -D CLANG_ENABLE_OBJC_REWRITER=ON \
-D LLVM_ENABLE_ASSERTIONS=ON \
-D LLVM_BUILD_EXAMPLES=ON \
-D COMPILER_RT_BUILD_LIBFUZZER=OFF \
diff --git a/clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp b/clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
index 8f275536b98a6..2b55820b38804 100644
--- a/clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
+++ b/clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
@@ -852,7 +852,7 @@ RewriteModernObjC::getIvarAccessString(ObjCIvarDecl *D) {
IvarT = GetGroupRecordTypeForObjCIvarBitfield(D);
if (!IvarT->getAs<TypedefType>() && IvarT->isRecordType()) {
- RecordDecl *RD = IvarT->castAs<RecordType>()->getDecl();
+ RecordDecl *RD = IvarT->castAs<RecordType>()->getOriginalDecl();
RD = RD->getDefinition();
if (RD && !RD->getDeclName().getAsIdentifierInfo()) {
// decltype(((Foo_IMPL*)0)->bar) *
@@ -865,7 +865,8 @@ RewriteModernObjC::getIvarAccessString(ObjCIvarDecl *D) {
RecordDecl *RD = RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(),
&Context->Idents.get(RecName));
- QualType PtrStructIMPL = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType PtrStructIMPL =
+ Context->getPointerType(Context->getCanonicalTagType(RD));
unsigned UnsignedIntSize =
static_cast<unsigned>(Context->getTypeSize(Context->UnsignedIntTy));
Expr *Zero = IntegerLiteral::Create(*Context,
@@ -2999,7 +3000,7 @@ QualType RewriteModernObjC::getSuperStructType() {
SuperStructDecl->completeDefinition();
}
- return Context->getTagDeclType(SuperStructDecl);
+ return Context->getCanonicalTagType(SuperStructDecl);
}
QualType RewriteModernObjC::getConstantStringStructType() {
@@ -3032,7 +3033,7 @@ QualType RewriteModernObjC::getConstantStringStructType() {
ConstantStringDecl->completeDefinition();
}
- return Context->getTagDeclType(ConstantStringDecl);
+ return Context->getCanonicalTagType(ConstantStringDecl);
}
/// getFunctionSourceLocation - returns start location of a function
@@ -3637,7 +3638,8 @@ bool RewriteModernObjC::RewriteObjCFieldDeclType(QualType &Type,
return RewriteObjCFieldDeclType(ElemTy, Result);
}
else if (Type->isRecordType()) {
- RecordDecl *RD = Type->castAs<RecordType>()->getDecl();
+ RecordDecl *RD =
+ Type->castAs<RecordType>()->getOriginalDecl()->getDefinitionOrSelf();
if (RD->isCompleteDefinition()) {
if (RD->isStruct())
Result += "\n\tstruct ";
@@ -3660,7 +3662,8 @@ bool RewriteModernObjC::RewriteObjCFieldDeclType(QualType &Type,
}
}
else if (Type->isEnumeralType()) {
- EnumDecl *ED = Type->castAs<EnumType>()->getDecl();
+ EnumDecl *ED =
+ Type->castAs<EnumType>()->getOriginalDecl()->getDefinitionOrSelf();
if (ED->isCompleteDefinition()) {
Result += "\n\tenum ";
Result += ED->getName();
@@ -3732,10 +3735,10 @@ void RewriteModernObjC::RewriteLocallyDefinedNamedAggregates(FieldDecl *fieldDec
TagDecl *TD = nullptr;
if (Type->isRecordType()) {
- TD = Type->castAs<RecordType>()->getDecl();
+ TD = Type->castAs<RecordType>()->getOriginalDecl()->getDefinitionOrSelf();
}
else if (Type->isEnumeralType()) {
- TD = Type->castAs<EnumType>()->getDecl();
+ TD = Type->castAs<EnumType>()->getOriginalDecl()->getDefinitionOrSelf();
}
if (TD) {
@@ -3793,7 +3796,7 @@ QualType RewriteModernObjC::SynthesizeBitfieldGroupStructType(
false, ICIS_NoInit));
}
RD->completeDefinition();
- return Context->getTagDeclType(RD);
+ return Context->getCanonicalTagType(RD);
}
QualType RewriteModernObjC::GetGroupRecordTypeForObjCIvarBitfield(ObjCIvarDecl *IV) {
@@ -4572,7 +4575,7 @@ Stmt *RewriteModernObjC::SynthesizeBlockCall(CallExpr *Exp, const Expr *BlockExp
RecordDecl *RD = RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(),
&Context->Idents.get("__block_impl"));
- QualType PtrBlock = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType PtrBlock = Context->getPointerType(Context->getCanonicalTagType(RD));
// Generate a funky cast.
SmallVector<QualType, 8> ArgTypes;
@@ -5316,7 +5319,8 @@ Stmt *RewriteModernObjC::SynthBlockInitExpr(BlockExpr *Exp,
RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(), II);
assert(RD && "SynthBlockInitExpr(): Can't find RecordDecl");
- QualType castT = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType castT =
+ Context->getPointerType(Context->getCanonicalTagType(RD));
FD = SynthBlockInitFunctionDecl(ND->getName());
Exp = new (Context) DeclRefExpr(*Context, FD, false, FD->getType(),
@@ -5719,7 +5723,10 @@ void RewriteModernObjC::HandleDeclInMainFile(Decl *D) {
}
}
} else if (VD->getType()->isRecordType()) {
- RecordDecl *RD = VD->getType()->castAs<RecordType>()->getDecl();
+ RecordDecl *RD = VD->getType()
+ ->castAs<RecordType>()
+ ->getOriginalDecl()
+ ->getDefinitionOrSelf();
if (RD->isCompleteDefinition())
RewriteRecordBody(RD);
}
@@ -7460,7 +7467,7 @@ Stmt *RewriteModernObjC::RewriteObjCIvarRefExpr(ObjCIvarRefExpr *IV) {
IvarT = GetGroupRecordTypeForObjCIvarBitfield(D);
if (!IvarT->getAs<TypedefType>() && IvarT->isRecordType()) {
- RecordDecl *RD = IvarT->castAs<RecordType>()->getDecl();
+ RecordDecl *RD = IvarT->castAs<RecordType>()->getOriginalDecl();
RD = RD->getDefinition();
if (RD && !RD->getDeclName().getAsIdentifierInfo()) {
// decltype(((Foo_IMPL*)0)->bar) *
@@ -7473,7 +7480,8 @@ Stmt *RewriteModernObjC::RewriteObjCIvarRefExpr(ObjCIvarRefExpr *IV) {
RecordDecl *RD = RecordDecl::Create(
*Context, TagTypeKind::Struct, TUDecl, SourceLocation(),
SourceLocation(), &Context->Idents.get(RecName));
- QualType PtrStructIMPL = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType PtrStructIMPL =
+ Context->getPointerType(Context->getCanonicalTagType(RD));
unsigned UnsignedIntSize =
static_cast<unsigned>(Context->getTypeSize(Context->UnsignedIntTy));
Expr *Zero = IntegerLiteral::Create(*Context,
diff --git a/clang/lib/Frontend/Rewrite/RewriteObjC.cpp b/clang/lib/Frontend/Rewrite/RewriteObjC.cpp
index f49ccf7be68e2..0242fc1f6e827 100644
--- a/clang/lib/Frontend/Rewrite/RewriteObjC.cpp
+++ b/clang/lib/Frontend/Rewrite/RewriteObjC.cpp
@@ -2358,7 +2358,7 @@ void RewriteObjC::SynthMsgSendSuperFunctionDecl() {
RecordDecl *RD = RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(),
&Context->Idents.get("objc_super"));
- QualType argT = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType argT = Context->getPointerType(Context->getCanonicalTagType(RD));
assert(!argT.isNull() && "Can't build 'struct objc_super *' type");
ArgTys.push_back(argT);
argT = Context->getObjCSelType();
@@ -2401,7 +2401,7 @@ void RewriteObjC::SynthMsgSendSuperStretFunctionDecl() {
RecordDecl *RD = RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(),
&Context->Idents.get("objc_super"));
- QualType argT = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType argT = Context->getPointerType(Context->getCanonicalTagType(RD));
assert(!argT.isNull() && "Can't build 'struct objc_super *' type");
ArgTys.push_back(argT);
argT = Context->getObjCSelType();
@@ -2552,7 +2552,7 @@ QualType RewriteObjC::getSuperStructType() {
SuperStructDecl->completeDefinition();
}
- return Context->getTagDeclType(SuperStructDecl);
+ return Context->getCanonicalTagType(SuperStructDecl);
}
QualType RewriteObjC::getConstantStringStructType() {
@@ -2585,7 +2585,7 @@ QualType RewriteObjC::getConstantStringStructType() {
ConstantStringDecl->completeDefinition();
}
- return Context->getTagDeclType(ConstantStringDecl);
+ return Context->getCanonicalTagType(ConstantStringDecl);
}
CallExpr *RewriteObjC::SynthMsgSendStretCallExpr(FunctionDecl *MsgSendStretFlavor,
@@ -3750,7 +3750,7 @@ Stmt *RewriteObjC::SynthesizeBlockCall(CallExpr *Exp, const Expr *BlockExp) {
RecordDecl *RD = RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(),
&Context->Idents.get("__block_impl"));
- QualType PtrBlock = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType PtrBlock = Context->getPointerType(Context->getCanonicalTagType(RD));
// Generate a funky cast.
SmallVector<QualType, 8> ArgTypes;
@@ -4468,7 +4468,8 @@ Stmt *RewriteObjC::SynthBlockInitExpr(BlockExpr *Exp,
RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(), II);
assert(RD && "SynthBlockInitExpr(): Can't find RecordDecl");
- QualType castT = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType castT =
+ Context->getPointerType(Context->getCanonicalTagType(RD));
FD = SynthBlockInitFunctionDecl((*I)->getName());
Exp = new (Context) DeclRefExpr(*Context, FD, false, FD->getType(),
@@ -4834,7 +4835,10 @@ void RewriteObjC::HandleDeclInMainFile(Decl *D) {
}
}
} else if (VD->getType()->isRecordType()) {
- RecordDecl *RD = VD->getType()->castAs<RecordType>()->getDecl();
+ RecordDecl *RD = VD->getType()
+ ->castAs<RecordType>()
+ ->getOriginalDecl()
+ ->getDefinitionOrSelf();
if (RD->isCompleteDefinition())
RewriteRecordBody(RD);
}
@@ -5804,7 +5808,8 @@ Stmt *RewriteObjCFragileABI::RewriteObjCIvarRefExpr(ObjCIvarRefExpr *IV) {
RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(), II);
assert(RD && "RewriteObjCIvarRefExpr(): Can't find RecordDecl");
- QualType castT = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType castT =
+ Context->getPointerType(Context->getCanonicalTagType(RD));
CastExpr *castExpr = NoTypeInfoCStyleCastExpr(Context, castT,
CK_BitCast,
IV->getBase());
@@ -5845,7 +5850,8 @@ Stmt *RewriteObjCFragileABI::RewriteObjCIvarRefExpr(ObjCIvarRefExpr *IV) {
RecordDecl::Create(*Context, TagTypeKind::Struct, TUDecl,
SourceLocation(), SourceLocation(), II);
assert(RD && "RewriteObjCIvarRefExpr(): Can't find RecordDecl");
- QualType castT = Context->getPointerType(Context->getTagDeclType(RD));
+ QualType castT =
+ Context->getPointerType(Context->getCanonicalTagType(RD));
CastExpr *castExpr = NoTypeInfoCStyleCastExpr(Context, castT,
CK_BitCast,
IV->getBase());
|
|
This default was changed in #119269 I asked for clarifications there. |
|
According to the author of the change, the feature is sort of obsolete and probably a waste of most users time to enable it. But if we are not removing it, then it needs testing, so its better to just enable it on CI anyway. Splitting the change would mean we'd either break all of linux pre-commit CI, and we would have to commit it and keep it in red until the second change lands, or we would need to commit this blind first without any testing, and only test it later. I'd have a slight preference to keep both changes together. |
|
If it's a waste of most user's time to enable it, it's probably a waste of premerge CI resources to test it there? Differing the configuration significantly between what people get locally by default and what runs in CI also creates a confusing user experience. Overall I think premerge should try and match the default configuration (which is theoretically what people care about given it's the default) as closely as possible. If people still feel strongly this should be off by default but also tested in premerge CI, we can RFC this and take it to the infrastructure area team if we can't reach a consensus. |
|
Alright, I'll remove the CI changes so we can unblock this. |
|
Though I note that we try to test any components that are actually modified, and this doesn't happen for the rewriter, so it gets no tests at all. |
This completes #147835 by adapting the changes in the API to the rewriter. Fixes build failures such as reported here: #147835 (comment) We previously missed this because it was not tested in pre-commit CI.
mizvekov
force-pushed
the
users/mizvekov/upd-RewriteModernObjC
branch
from
df70cd6 to
70a2c65
Compare
|
I bypassed the rules to merge because the component is not tested at all, it would be a waste of a CI run. |
We do this at the project level rather than at the level of individual options. The only exception we make is for ClangIR which is in a bit of a unique position in that there is very active upstream development but it is not enabled by default. |
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 completes #147835 by adapting the changes in the API to the rewriter.
Fixes build failures such as reported here: #147835 (comment)
We previously missed this because it was not tested in pre-commit CI. This patch also addresses that problem.