zixuw created this revision.
Herald added a subscriber: arphaman.
Herald added a reviewer: dang.
Herald added a project: All.
zixuw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
The current implementation in `getRawCommentForDeclNoCacheImpl` to check
if a leading comment is attached to a given `Decl *D` is to see if there
is any of `";{}#@"` between the comment and the source location of `D`,
which indicates that there's other declarations or preprocessor
directives that the comment is meant for. However, this does not work
for enum constants, which are separated by commas.
Comma was originally included in the filter characters list, but removed
in b534d3a0ef69 because macros, attributes and other decorators might
also introduce commas before a declaration. And there's no good way to
reliably and efficiently check if all the commas are from other
declarations.
This patch added a forward pointer in `DeclBase` pointing to the
previous declaration inside the lexical `DeclContext`, similar to the
existing `NextInContextAndBits`. This constructs a doubly linked list so
we can easily query the lexically preceding declaration of the current
decl `D`, to check if any decl falls between the comment and `D`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125061
Files:
clang/include/clang/AST/DeclBase.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/DeclBase.cpp
clang/test/Index/annotate-comments-enum-constant.c
clang/test/Index/annotate-comments.cpp
Index: clang/test/Index/annotate-comments.cpp
===================================================================
--- clang/test/Index/annotate-comments.cpp
+++ clang/test/Index/annotate-comments.cpp
@@ -254,6 +254,14 @@
MYMAC(0,0)
void isdoxy54(int);
+/// IS_DOXYGEN_SINGLE
+struct isdoxy55 {
+ int notdoxy48;
+ /// IS_DOXYGEN_SINGLE
+ int isdoxy56;
+};
+bool notdoxy49;
+
#endif
// RUN: rm -rf %t
@@ -337,3 +345,6 @@
// CHECK: annotate-comments.cpp:241:6: FunctionDecl=isdoxy52:{{.*}} BriefComment=[Aaa. IS_DOXYGEN_START Bbb.]
// CHECK: annotate-comments.cpp:248:6: FunctionDecl=isdoxy53:{{.*}} BriefComment=[Aaa. IS_DOXYGEN_START IS_DOXYGEN_END]
// CHECK: annotate-comments.cpp:255:6: FunctionDecl=isdoxy54:{{.*}} BriefComment=[Aaa. IS_DOXYGEN_START IS_DOXYGEN_END]
+
+// CHECK: annotate-comments.cpp:258:8: StructDecl=isdoxy55:{{.*}} IS_DOXYGEN_SINGLE
+// CHECK: annotate-comments.cpp:261:7: FieldDecl=isdoxy56:{{.*}} IS_DOXYGEN_SINGLE
Index: clang/test/Index/annotate-comments-enum-constant.c
===================================================================
--- /dev/null
+++ clang/test/Index/annotate-comments-enum-constant.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: c-index-test -test-load-source all -comments-xml-schema=%S/../../bindings/xml/comment-xml-schema.rng %s > %t/out
+// RUN: FileCheck %s --dump-input always < %t/out
+
+enum {
+ /// Documentation for Foo
+ Foo,
+ Bar, // No documentation for Bar
+ /// Documentation for Baz
+ Baz,
+};
+// CHECK: EnumConstantDecl=Foo:[[@LINE-5]]:3 (Definition) {{.*}} BriefComment=[Documentation for Foo] FullCommentAsHTML=[<p class="para-brief"> Documentation for Foo</p>] FullCommentAsXML=[<Variable file="{{[^"]+}}annotate-comments-enum-constant.c" line="[[@LINE-5]]" column="3"><Name>Foo</Name><USR>c:@Ea@Foo@Foo</USR><Declaration>Foo</Declaration><Abstract><Para> Documentation for Foo</Para></Abstract></Variable>]
+// CHECK: EnumConstantDecl=Bar:[[@LINE-5]]:3 (Definition)
+// CHECK-NOT: BriefComment=[Documentation for Foo]
+// CHECK: EnumConstantDecl=Baz:[[@LINE-5]]:3 (Definition) {{.*}} BriefComment=[Documentation for Baz] FullCommentAsHTML=[<p class="para-brief"> Documentation for Baz</p>] FullCommentAsXML=[<Variable file="{{[^"]+}}annotate-comments-enum-constant.c" line="[[@LINE-5]]" column="3"><Name>Baz</Name><USR>c:@Ea@Foo@Baz</USR><Declaration>Baz</Declaration><Abstract><Para> Documentation for Baz</Para></Abstract></Variable>]
Index: clang/lib/AST/DeclBase.cpp
===================================================================
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1344,6 +1344,7 @@
else
FirstNewDecl = D;
+ D->PreviousInContext = PrevDecl;
PrevDecl = D;
}
@@ -1392,6 +1393,8 @@
std::tie(ExternalFirst, ExternalLast) =
BuildDeclChain(Decls, FieldsAlreadyLoaded);
ExternalLast->NextInContextAndBits.setPointer(FirstDecl);
+ if (FirstDecl)
+ FirstDecl->PreviousInContext = ExternalLast;
FirstDecl = ExternalFirst;
if (!LastDecl)
LastDecl = ExternalLast;
@@ -1498,25 +1501,26 @@
assert((D->NextInContextAndBits.getPointer() || D == LastDecl) &&
"decl is not in decls list");
- // Remove D from the decl chain. This is O(n) but hopefully rare.
+ // Remove D from the decl chain.
if (D == FirstDecl) {
if (D == LastDecl)
FirstDecl = LastDecl = nullptr;
else
FirstDecl = D->NextInContextAndBits.getPointer();
- } else {
- for (Decl *I = FirstDecl; true; I = I->NextInContextAndBits.getPointer()) {
- assert(I && "decl not found in linked list");
- if (I->NextInContextAndBits.getPointer() == D) {
- I->NextInContextAndBits.setPointer(D->NextInContextAndBits.getPointer());
- if (D == LastDecl) LastDecl = I;
- break;
- }
- }
+ }
+
+ if (auto *Next = D->NextInContextAndBits.getPointer())
+ Next->PreviousInContext = D->PreviousInContext;
+
+ if (auto *Prev = D->PreviousInContext) {
+ Prev->NextInContextAndBits.setPointer(D->NextInContextAndBits.getPointer());
+ if (D == LastDecl)
+ LastDecl = Prev;
}
// Mark that D is no longer in the decl chain.
D->NextInContextAndBits.setPointer(nullptr);
+ D->PreviousInContext = nullptr;
// Remove D from the lookup table if necessary.
if (isa<NamedDecl>(D)) {
@@ -1551,6 +1555,7 @@
if (FirstDecl) {
LastDecl->NextInContextAndBits.setPointer(D);
+ D->PreviousInContext = LastDecl;
LastDecl = D;
} else {
FirstDecl = LastDecl = D;
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -258,6 +258,44 @@
CommentBeforeDecl->isTrailingComment())
return nullptr;
+ const SourceLocation &CommentEndLoc = CommentBeforeDecl->getEndLoc();
+ if (CommentEndLoc.isInvalid())
+ return nullptr;
+
+ // Check if there's any other declaration in between the end of the comment
+ // and the current declaration.
+ if (const Decl *PreviousInContext = D->getPreviousDeclInContext()) {
+ // Use the previous decl's declaration location if the previous declaration
+ // is the lexical declaration context of the current declaration, and the
+ // end location otherwise. For example:
+ //
+ // ```
+ // /// Doc for struct s
+ // struct s { // <- declaration location for `s`
+ //
+ // int i; // Both the PreviousDeclInContext and the
+ // // LexicalDeclContext of `i` is `s`
+ //
+ // /// Doc for func
+ // void func();
+ // }; // <- end location for `s`
+ //
+ // bool b; // The PreviousDeclInContext of `b` is `s`
+ // ```
+ //
+ // In this case, `Doc for struct s` shouldn't be attached to `i`; and
+ // `Doc for func` shouldn't be attached to `b`.
+ const DeclContext *DC = dyn_cast<DeclContext>(PreviousInContext);
+ const SourceLocation &PreviousDeclLoc =
+ DC && D->getLexicalDeclContext() == DC
+ ? PreviousInContext->getLocation()
+ : PreviousInContext->getEndLoc();
+ if (PreviousDeclLoc.isValid() && PreviousDeclLoc.isFileID() &&
+ SourceMgr.isPointWithin(PreviousDeclLoc, CommentEndLoc,
+ RepresentativeLocForDecl))
+ return nullptr;
+ }
+
// Decompose the end of the comment.
const unsigned CommentEndOffset =
Comments.getCommentEndOffset(CommentBeforeDecl);
@@ -273,9 +311,8 @@
StringRef Text(Buffer + CommentEndOffset,
DeclLocDecomp.second - CommentEndOffset);
- // There should be no other declarations or preprocessor directives between
- // comment and declaration.
- if (Text.find_first_of(";{}#@") != StringRef::npos)
+ // There should be no preprocessor directives between comment and declaration.
+ if (Text.find_first_of("#@") != StringRef::npos)
return nullptr;
return CommentBeforeDecl;
Index: clang/include/clang/AST/DeclBase.h
===================================================================
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -238,6 +238,9 @@
/// The extra two bits are used for the ModuleOwnershipKind.
llvm::PointerIntPair<Decl *, 2, ModuleOwnershipKind> NextInContextAndBits;
+ /// The previous declaration within the same lexical DeclContext.
+ Decl *PreviousInContext;
+
private:
friend class DeclContext;
@@ -374,21 +377,23 @@
protected:
Decl(Kind DK, DeclContext *DC, SourceLocation L)
: NextInContextAndBits(nullptr, getModuleOwnershipKindForChildOf(DC)),
- DeclCtx(DC), Loc(L), DeclKind(DK), InvalidDecl(false), HasAttrs(false),
- Implicit(false), Used(false), Referenced(false),
- TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0),
- IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
+ PreviousInContext(nullptr), DeclCtx(DC), Loc(L), DeclKind(DK),
+ InvalidDecl(false), HasAttrs(false), Implicit(false), Used(false),
+ Referenced(false), TopLevelDeclInObjCContainer(false), Access(AS_none),
+ FromASTFile(0), IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
CacheValidAndLinkage(0) {
- if (StatisticsEnabled) add(DK);
+ if (StatisticsEnabled)
+ add(DK);
}
Decl(Kind DK, EmptyShell Empty)
- : DeclKind(DK), InvalidDecl(false), HasAttrs(false), Implicit(false),
- Used(false), Referenced(false), TopLevelDeclInObjCContainer(false),
- Access(AS_none), FromASTFile(0),
+ : PreviousInContext(nullptr), DeclKind(DK), InvalidDecl(false),
+ HasAttrs(false), Implicit(false), Used(false), Referenced(false),
+ TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0),
IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
CacheValidAndLinkage(0) {
- if (StatisticsEnabled) add(DK);
+ if (StatisticsEnabled)
+ add(DK);
}
virtual ~Decl();
@@ -431,6 +436,9 @@
Decl *getNextDeclInContext() { return NextInContextAndBits.getPointer(); }
const Decl *getNextDeclInContext() const {return NextInContextAndBits.getPointer();}
+ Decl *getPreviousDeclInContext() { return PreviousInContext; }
+ const Decl *getPreviousDeclInContext() const { return PreviousInContext; }
+
DeclContext *getDeclContext() {
if (isInSemaDC())
return getSemanticDC();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
