Skip to content

Commit

Permalink
Fix de-nesting of unnamed TagDecls and improve some comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
mattmccutchen-cci committed Sep 1, 2021
1 parent a4cf721 commit 4ce2cfb
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 12 deletions.
35 changes: 25 additions & 10 deletions clang/lib/3C/DeclRewriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,30 @@ void DeclRewriter::denestTagDecls() {
std::string DefinitionStr = R.getRewrittenText(CSR);
// Delete the definition from the old location.
rewriteSourceRange(R, CSR, "");
// We want to insert RD as a new child of its original semantic DeclContext,
// just before the existing child of that DeclContext of which RD was
// originally a descendant.
DeclContext *TopChild = TD;
while (TopChild->getLexicalParent() != TD->getDeclContext()) {
TopChild = TopChild->getLexicalParent();
// We want to find the nearest ancestor DeclContext of TD that is _not_ a
// TagDecl and make TD a child of that DeclContext (named `Parent` below),
// just before the child `TopTagDecl` of `Parent` of which TD was originally
// a descendant.
//
// As of this writing, it seems that if TD is named, we get
// `Parent == TD->getDeclContext()` due to the code at
// https://github.com/correctcomputation/checkedc-clang/blob/fd4d8af4383d40af10ee8bc92b7bf88061a11035/clang/lib/Sema/SemaDecl.cpp#L16980-L16981,
// But that code doesn't run if TD is unnamed (which makes some sense
// because name visibility isn't an issue for TagDecls that have no name),
// and we want to de-nest TagDecls with names we assigned just like ones
// that were originally named, so we can't just use `TD->getDeclContext()`.
// In any event, maybe we wouldn't want to rely on this kind of internal
// Clang behavior.
DeclContext *TopTagDecl = TD;
for (;;) {
DeclContext *Parent = TopTagDecl->getLexicalParent();
if (!isa<TagDecl>(Parent))
break;
TopTagDecl = Parent;
}
// TODO: Use a wrapper like rewriteSourceRange.
R.InsertText(cast<Decl>(TopChild)->getBeginLoc(), DefinitionStr);
// TODO: Use a wrapper like rewriteSourceRange that tries harder with
// macros, reports failure, etc.
R.InsertText(cast<Decl>(TopTagDecl)->getBeginLoc(), DefinitionStr);
}
}

Expand Down Expand Up @@ -332,8 +347,8 @@ void DeclRewriter::rewriteMultiDecl(MultiDeclInfo &MDI, RSet &ToRewrite) {
llvm_unreachable("Failed to find place to insert assigned TagDecl name.");
}
}
// Make a note if the RecordDecl needs to be de-nested later.
if (TD->getLexicalDeclContext() != TD->getDeclContext())
// Make a note if the TagDecl needs to be de-nested later.
if (isa<TagDecl>(TD->getLexicalDeclContext()))
TagDeclsToDenest.push_back(TD);
// `struct T { ... } foo;` -> `struct T { ... };\nfoo;`
rewriteSourceRange(R, TD->getEndLoc(), "};\n");
Expand Down
13 changes: 11 additions & 2 deletions clang/lib/3C/MultiDecls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ void MultiDeclsInfo::findMultiDecls(DeclContext *DC, ASTContext &Context) {
// Otherwise, just generate a new tag name based on the member name.
// Example: `struct { ... } foo;` ->
// `struct foo_struct_1 { ... }; struct foo_struct_1 foo;`
// If `foo_struct_1` is already taken, use `foo_struct_2`, etc.
std::string KindName = std::string(LastTagDef->getKindName());
std::string NewName;
for (int Num = 1; ; Num++) {
Expand All @@ -142,8 +143,16 @@ void MultiDeclsInfo::findMultiDecls(DeclContext *DC, ASTContext &Context) {
}
AssignedTagTypeStrs.insert(std::make_pair(TagDefPSL, KindName + " " + NewName));
TagDefNeedsName = false;
// This name is now taken; other automatically generated names must not
// collide with it.
// Consider this name taken and ensure that other automatically
// generated names do not collide with it.
//
// If the multi-decl doesn't end up getting rewritten, this name
// ultimately may not be used, creating a gap in the numbering in 3C's
// output. But this cosmetic inconsistency is a small price to pay for
// the architectural convenience of being able to store the assigned
// names in the PointerVariableConstraints when they are constructed
// rather than trying to assign and store the names after we know
// which multi-decls will be rewritten.
UsedTagNames.insert(NewName);
}
}
Expand Down

0 comments on commit 4ce2cfb

Please sign in to comment.