Thanks for the review. I have attached new version of the patch.
I looked why there were no test failures:
the off-by-one error in Sema::BuildDeclaratorGroup had no negativ effect
(besides small runtime penalty) because you need minimum two decls to be
different for a diagnostic.
The Group.slice(1) in Sema::ActOnDocumetableDecls is called in testsuite
at typedefs but was not able to create a doxygen comments where it
makes a difference.
On Fri, 2013-05-17 at 10:39 -0700, David Blaikie wrote:
>
>
>
> On Fri, May 17, 2013 at 6:31 AM, Rafael Espíndola
> <[email protected]> wrote:
> On 11 May 2013 15:26, Robert Wilhelm <[email protected]>
> wrote:
> > This patch converts three methods to ArrayRef in Sema.
> > No functionality change.
> > Passes make test on x86_64-unknown-linux-gnu
>
>
> - if (TypeMayContainAuto && NumDecls > 1) {
> + if (TypeMayContainAuto && !Group.empty()) {
>
> Should be 'Group.size() > 1' no?
>
> - Group++;
> - NumDecls--;
> + Group.slice(1);
>
> slice returns a new array ref, it doesn't modify the current
> one.
>
>
> & if these mistakes weren't causing test failures - perhaps you could
> figure out which tests are missing & add them :)
>
> - David
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 182516)
+++ include/clang/Sema/Sema.h (working copy)
@@ -1478,15 +1478,14 @@
void SetDeclDefaulted(Decl *dcl, SourceLocation DefaultLoc);
void FinalizeDeclaration(Decl *D);
DeclGroupPtrTy FinalizeDeclaratorGroup(Scope *S, const DeclSpec &DS,
- Decl **Group,
- unsigned NumDecls);
- DeclGroupPtrTy BuildDeclaratorGroup(Decl **Group, unsigned NumDecls,
+ ArrayRef<Decl *> Group);
+ DeclGroupPtrTy BuildDeclaratorGroup(llvm::MutableArrayRef<Decl *> Group,
bool TypeMayContainAuto = true);
/// Should be called on all declarations that might have attached
/// documentation comments.
void ActOnDocumentableDecl(Decl *D);
- void ActOnDocumentableDecls(Decl **Group, unsigned NumDecls);
+ void ActOnDocumentableDecls(ArrayRef<Decl *> Group);
void ActOnFinishKNRParamDeclarations(Scope *S, Declarator &D,
SourceLocation LocAfterDecls);
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp (revision 182516)
+++ lib/Parse/ParseDecl.cpp (working copy)
@@ -1666,7 +1666,7 @@
Actions.ActOnCXXForRangeDecl(ThisDecl);
Actions.FinalizeDeclaration(ThisDecl);
D.complete(ThisDecl);
- return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, &ThisDecl, 1);
+ return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, ThisDecl);
}
SmallVector<Decl *, 8> DeclsInGroup;
@@ -1733,9 +1733,7 @@
}
}
- return Actions.FinalizeDeclaratorGroup(getCurScope(), DS,
- DeclsInGroup.data(),
- DeclsInGroup.size());
+ return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, DeclsInGroup);
}
/// Parse an optional simple-asm-expr and attributes, and attach them to a
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp (revision 182516)
+++ lib/Parse/ParseDeclCXX.cpp (working copy)
@@ -2349,8 +2349,7 @@
return;
}
- Actions.FinalizeDeclaratorGroup(getCurScope(), DS, DeclsInGroup.data(),
- DeclsInGroup.size());
+ Actions.FinalizeDeclaratorGroup(getCurScope(), DS, DeclsInGroup);
}
/// ParseCXXMemberInitializer - Parse the brace-or-equal-initializer or
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp (revision 182516)
+++ lib/Parse/ParseStmt.cpp (working copy)
@@ -818,7 +818,7 @@
DeclSpec DS(AttrFactory);
DeclGroupPtrTy Res = Actions.FinalizeDeclaratorGroup(getCurScope(), DS,
- DeclsInGroup.data(), DeclsInGroup.size());
+ DeclsInGroup);
StmtResult R = Actions.ActOnDeclStmt(Res, LabelLoc, Tok.getLocation());
ExpectAndConsumeSemi(diag::err_expected_semi_declaration);
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp (revision 182516)
+++ lib/Sema/SemaDecl.cpp (working copy)
@@ -8202,13 +8202,13 @@
Sema::DeclGroupPtrTy
Sema::FinalizeDeclaratorGroup(Scope *S, const DeclSpec &DS,
- Decl **Group, unsigned NumDecls) {
+ ArrayRef<Decl *> Group) {
SmallVector<Decl*, 8> Decls;
if (DS.isTypeSpecOwned())
Decls.push_back(DS.getRepAsDecl());
- for (unsigned i = 0; i != NumDecls; ++i)
+ for (unsigned i = 0, e = Group.size(); i != e; ++i)
if (Decl *D = Group[i])
Decls.push_back(D);
@@ -8216,14 +8216,13 @@
if (const TagDecl *Tag = dyn_cast_or_null<TagDecl>(DS.getRepAsDecl()))
getASTContext().addUnnamedTag(Tag);
- return BuildDeclaratorGroup(Decls.data(), Decls.size(),
- DS.containsPlaceholderType());
+ return BuildDeclaratorGroup(Decls, DS.containsPlaceholderType());
}
/// BuildDeclaratorGroup - convert a list of declarations into a declaration
/// group, performing any necessary semantic checking.
Sema::DeclGroupPtrTy
-Sema::BuildDeclaratorGroup(Decl **Group, unsigned NumDecls,
+Sema::BuildDeclaratorGroup(llvm::MutableArrayRef<Decl *> Group,
bool TypeMayContainAuto) {
// C++0x [dcl.spec.auto]p7:
// If the type deduced for the template parameter U is not the same in each
@@ -8232,11 +8231,11 @@
// between the deduced type U and the deduced type which 'auto' stands for.
// auto a = 0, b = { 1, 2, 3 };
// is legal because the deduced type U is 'int' in both cases.
- if (TypeMayContainAuto && NumDecls > 1) {
+ if (TypeMayContainAuto && Group.size() > 1) {
QualType Deduced;
CanQualType DeducedCanon;
VarDecl *DeducedDecl = 0;
- for (unsigned i = 0; i != NumDecls; ++i) {
+ for (unsigned i = 0, e = Group.size(); i != e; ++i) {
if (VarDecl *D = dyn_cast<VarDecl>(Group[i])) {
AutoType *AT = D->getType()->getContainedAutoType();
// Don't reissue diagnostics when instantiating a template.
@@ -8265,18 +8264,19 @@
}
}
- ActOnDocumentableDecls(Group, NumDecls);
+ ActOnDocumentableDecls(Group);
- return DeclGroupPtrTy::make(DeclGroupRef::Create(Context, Group, NumDecls));
+ return DeclGroupPtrTy::make(DeclGroupRef::Create(Context, Group.data(),
+ Group.size()));
}
void Sema::ActOnDocumentableDecl(Decl *D) {
- ActOnDocumentableDecls(&D, 1);
+ ActOnDocumentableDecls(D);
}
-void Sema::ActOnDocumentableDecls(Decl **Group, unsigned NumDecls) {
+void Sema::ActOnDocumentableDecls(ArrayRef<Decl *> Group) {
// Don't parse the comment if Doxygen diagnostics are ignored.
- if (NumDecls == 0 || !Group[0])
+ if (Group.empty() || !Group[0])
return;
if (Diags.getDiagnosticLevel(diag::warn_doc_param_not_found,
@@ -8284,9 +8284,9 @@
== DiagnosticsEngine::Ignored)
return;
- if (NumDecls >= 2) {
+ if (Group.size() >= 2) {
// This is a decl group. Normally it will contain only declarations
- // procuded from declarator list. But in case we have any definitions or
+ // produced from declarator list. But in case we have any definitions or
// additional declaration references:
// 'typedef struct S {} S;'
// 'typedef struct S *S;'
@@ -8294,8 +8294,7 @@
// FinalizeDeclaratorGroup adds these as separate declarations.
Decl *MaybeTagDecl = Group[0];
if (MaybeTagDecl && isa<TagDecl>(MaybeTagDecl)) {
- Group++;
- NumDecls--;
+ Group = Group.slice(1);
}
}
@@ -8310,7 +8309,7 @@
// declaration, but also comments that *follow* the declaration -- thanks to
// the lookahead in the lexer: we've consumed the semicolon and looked
// ahead through comments.
- for (unsigned i = 0; i != NumDecls; ++i)
+ for (unsigned i = 0, e = Group.size(); i != e; ++i)
Context.getCommentForDecl(Group[i], &PP);
}
}
Index: lib/Sema/SemaDeclObjC.cpp
===================================================================
--- lib/Sema/SemaDeclObjC.cpp (revision 182516)
+++ lib/Sema/SemaDeclObjC.cpp (working copy)
@@ -819,7 +819,7 @@
DeclsInGroup.push_back(PDecl);
}
- return BuildDeclaratorGroup(DeclsInGroup.data(), DeclsInGroup.size(), false);
+ return BuildDeclaratorGroup(DeclsInGroup, false);
}
Decl *Sema::
@@ -1084,7 +1084,7 @@
DeclsInGroup.push_back(ObjCImpDecl);
- return BuildDeclaratorGroup(DeclsInGroup.data(), DeclsInGroup.size(), false);
+ return BuildDeclaratorGroup(DeclsInGroup, false);
}
void Sema::CheckImplementationIvars(ObjCImplementationDecl *ImpDecl,
@@ -1946,7 +1946,7 @@
DeclsInGroup.push_back(IDecl);
}
- return BuildDeclaratorGroup(DeclsInGroup.data(), DeclsInGroup.size(), false);
+ return BuildDeclaratorGroup(DeclsInGroup, false);
}
static bool tryMatchRecordTypes(ASTContext &Context,
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp (revision 182516)
+++ lib/Sema/SemaStmt.cpp (working copy)
@@ -1766,7 +1766,8 @@
// Claim the type doesn't contain auto: we've already done the checking.
DeclGroupPtrTy RangeGroup =
- BuildDeclaratorGroup((Decl**)&RangeVar, 1, /*TypeMayContainAuto=*/false);
+ BuildDeclaratorGroup( llvm::MutableArrayRef<Decl *>((Decl **)&RangeVar,1),
+ /*TypeMayContainAuto=*/false);
StmtResult RangeDecl = ActOnDeclStmt(RangeGroup, RangeLoc, RangeLoc);
if (RangeDecl.isInvalid())
return StmtError();
@@ -2046,7 +2047,8 @@
Decl *BeginEndDecls[] = { BeginVar, EndVar };
// Claim the type doesn't contain auto: we've already done the checking.
DeclGroupPtrTy BeginEndGroup =
- BuildDeclaratorGroup(BeginEndDecls, 2, /*TypeMayContainAuto=*/false);
+ BuildDeclaratorGroup( llvm::MutableArrayRef<Decl *>(BeginEndDecls, 2),
+ /*TypeMayContainAuto=*/false);
BeginEndDecl = ActOnDeclStmt(BeginEndGroup, ColonLoc, ColonLoc);
const QualType BeginRefNonRefType = BeginType.getNonReferenceType();
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h (revision 182516)
+++ lib/Sema/TreeTransform.h (working copy)
@@ -1163,10 +1163,10 @@
///
/// By default, performs semantic analysis to build the new statement.
/// Subclasses may override this routine to provide different behavior.
- StmtResult RebuildDeclStmt(Decl **Decls, unsigned NumDecls,
+ StmtResult RebuildDeclStmt(llvm::MutableArrayRef<Decl *> Decls,
SourceLocation StartLoc,
SourceLocation EndLoc) {
- Sema::DeclGroupPtrTy DG = getSema().BuildDeclaratorGroup(Decls, NumDecls);
+ Sema::DeclGroupPtrTy DG = getSema().BuildDeclaratorGroup(Decls);
return getSema().ActOnDeclStmt(DG, StartLoc, EndLoc);
}
@@ -5580,8 +5580,7 @@
if (!getDerived().AlwaysRebuild() && !DeclChanged)
return SemaRef.Owned(S);
- return getDerived().RebuildDeclStmt(Decls.data(), Decls.size(),
- S->getStartLoc(), S->getEndLoc());
+ return getDerived().RebuildDeclStmt(Decls, S->getStartLoc(), S->getEndLoc());
}
template<typename Derived>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
