Attaching comments to redeclarations: fix wrong assumptions
The reason for the recent fallout for "attaching comments to any redeclaration" change are two false assumptions: (1) a RawComment is attached to a single decl (not true for 'typedef struct X *Y' where we want the comment to be attached to both X and Y); (2) the whole redeclaration chain has only a single comment (obviously false, the user can put a separate comment for each redeclaration). To fix (1) I revert the part of the recent change where a 'Decl*' member was introduced to RawComment. Now ASTContext has a separate DenseMap for mapping 'Decl*' to 'FullComment*'. To fix (2) I just removed the test with this assumption. We might not parse every comment in redecl chain if we already parsed at least one. llvm-svn: 161878
This commit is contained in:
parent
378b292aab
commit
b261088a61
|
@ -485,6 +485,10 @@ public:
|
||||||
/// lazily.
|
/// lazily.
|
||||||
mutable llvm::DenseMap<const Decl *, RawCommentAndCacheFlags> RedeclComments;
|
mutable llvm::DenseMap<const Decl *, RawCommentAndCacheFlags> RedeclComments;
|
||||||
|
|
||||||
|
/// \brief Mapping from declarations to parsed comments attached to any
|
||||||
|
/// redeclaration.
|
||||||
|
mutable llvm::DenseMap<const Decl *, comments::FullComment *> ParsedComments;
|
||||||
|
|
||||||
/// \brief Return the documentation comment attached to a given declaration,
|
/// \brief Return the documentation comment attached to a given declaration,
|
||||||
/// without looking into cache.
|
/// without looking into cache.
|
||||||
RawComment *getRawCommentForDeclNoCache(const Decl *D) const;
|
RawComment *getRawCommentForDeclNoCache(const Decl *D) const;
|
||||||
|
|
|
@ -55,16 +55,11 @@ public:
|
||||||
|
|
||||||
/// Is this comment attached to any declaration?
|
/// Is this comment attached to any declaration?
|
||||||
bool isAttached() const LLVM_READONLY {
|
bool isAttached() const LLVM_READONLY {
|
||||||
return !DeclOrParsedComment.isNull();
|
return IsAttached;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Return the declaration that this comment is attached to.
|
void setAttached() {
|
||||||
const Decl *getDecl() const;
|
IsAttached = true;
|
||||||
|
|
||||||
/// Set the declaration that this comment is attached to.
|
|
||||||
void setDecl(const Decl *D) {
|
|
||||||
assert(DeclOrParsedComment.isNull());
|
|
||||||
DeclOrParsedComment = D;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns true if it is a comment that should be put after a member:
|
/// Returns true if it is a comment that should be put after a member:
|
||||||
|
@ -118,28 +113,23 @@ public:
|
||||||
return extractBriefText(Context);
|
return extractBriefText(Context);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns a \c FullComment AST node, parsing the comment if needed.
|
/// Parse the comment, assuming it is attached to decl \c D.
|
||||||
comments::FullComment *getParsed(const ASTContext &Context) const {
|
comments::FullComment *parse(const ASTContext &Context, const Decl *D) const;
|
||||||
if (comments::FullComment *FC =
|
|
||||||
DeclOrParsedComment.dyn_cast<comments::FullComment *>())
|
|
||||||
return FC;
|
|
||||||
|
|
||||||
return parse(Context);
|
|
||||||
}
|
|
||||||
|
|
||||||
private:
|
private:
|
||||||
SourceRange Range;
|
SourceRange Range;
|
||||||
|
|
||||||
mutable StringRef RawText;
|
mutable StringRef RawText;
|
||||||
mutable const char *BriefText;
|
mutable const char *BriefText;
|
||||||
mutable llvm::PointerUnion<const Decl *, comments::FullComment *>
|
|
||||||
DeclOrParsedComment;
|
|
||||||
|
|
||||||
mutable bool RawTextValid : 1; ///< True if RawText is valid
|
mutable bool RawTextValid : 1; ///< True if RawText is valid
|
||||||
mutable bool BriefTextValid : 1; ///< True if BriefText is valid
|
mutable bool BriefTextValid : 1; ///< True if BriefText is valid
|
||||||
|
|
||||||
unsigned Kind : 3;
|
unsigned Kind : 3;
|
||||||
|
|
||||||
|
/// True if comment is attached to a declaration in ASTContext.
|
||||||
|
bool IsAttached : 1;
|
||||||
|
|
||||||
bool IsTrailingComment : 1;
|
bool IsTrailingComment : 1;
|
||||||
bool IsAlmostTrailingComment : 1;
|
bool IsAlmostTrailingComment : 1;
|
||||||
|
|
||||||
|
@ -152,7 +142,7 @@ private:
|
||||||
RawComment(SourceRange SR, CommentKind K, bool IsTrailingComment,
|
RawComment(SourceRange SR, CommentKind K, bool IsTrailingComment,
|
||||||
bool IsAlmostTrailingComment) :
|
bool IsAlmostTrailingComment) :
|
||||||
Range(SR), RawTextValid(false), BriefTextValid(false), Kind(K),
|
Range(SR), RawTextValid(false), BriefTextValid(false), Kind(K),
|
||||||
IsTrailingComment(IsTrailingComment),
|
IsAttached(false), IsTrailingComment(IsTrailingComment),
|
||||||
IsAlmostTrailingComment(IsAlmostTrailingComment),
|
IsAlmostTrailingComment(IsAlmostTrailingComment),
|
||||||
BeginLineValid(false), EndLineValid(false)
|
BeginLineValid(false), EndLineValid(false)
|
||||||
{ }
|
{ }
|
||||||
|
@ -161,8 +151,6 @@ private:
|
||||||
|
|
||||||
const char *extractBriefText(const ASTContext &Context) const;
|
const char *extractBriefText(const ASTContext &Context) const;
|
||||||
|
|
||||||
comments::FullComment *parse(const ASTContext &Context) const;
|
|
||||||
|
|
||||||
friend class ASTReader;
|
friend class ASTReader;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
@ -66,6 +66,12 @@ RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
|
||||||
if (D->isImplicit())
|
if (D->isImplicit())
|
||||||
return NULL;
|
return NULL;
|
||||||
|
|
||||||
|
// User can not attach documentation to implicit instantiations.
|
||||||
|
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
|
||||||
|
if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
|
|
||||||
// TODO: handle comments for function parameters properly.
|
// TODO: handle comments for function parameters properly.
|
||||||
if (isa<ParmVarDecl>(D))
|
if (isa<ParmVarDecl>(D))
|
||||||
return NULL;
|
return NULL;
|
||||||
|
@ -145,7 +151,6 @@ RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
|
||||||
SourceMgr.getLineNumber(DeclLocDecomp.first, DeclLocDecomp.second)
|
SourceMgr.getLineNumber(DeclLocDecomp.first, DeclLocDecomp.second)
|
||||||
== SourceMgr.getLineNumber(CommentBeginDecomp.first,
|
== SourceMgr.getLineNumber(CommentBeginDecomp.first,
|
||||||
CommentBeginDecomp.second)) {
|
CommentBeginDecomp.second)) {
|
||||||
(*Comment)->setDecl(D);
|
|
||||||
return *Comment;
|
return *Comment;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -185,13 +190,13 @@ RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
|
||||||
if (Text.find_first_of(",;{}#@") != StringRef::npos)
|
if (Text.find_first_of(",;{}#@") != StringRef::npos)
|
||||||
return NULL;
|
return NULL;
|
||||||
|
|
||||||
(*Comment)->setDecl(D);
|
|
||||||
return *Comment;
|
return *Comment;
|
||||||
}
|
}
|
||||||
|
|
||||||
const RawComment *ASTContext::getRawCommentForAnyRedecl(const Decl *D) const {
|
namespace {
|
||||||
// If we have a 'templated' declaration for a template, adjust 'D' to
|
/// If we have a 'templated' declaration for a template, adjust 'D' to
|
||||||
// refer to the actual template.
|
/// refer to the actual template.
|
||||||
|
const Decl *adjustDeclToTemplate(const Decl *D) {
|
||||||
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
|
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
|
||||||
if (const FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate())
|
if (const FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate())
|
||||||
D = FTD;
|
D = FTD;
|
||||||
|
@ -200,6 +205,12 @@ const RawComment *ASTContext::getRawCommentForAnyRedecl(const Decl *D) const {
|
||||||
D = CTD;
|
D = CTD;
|
||||||
}
|
}
|
||||||
// FIXME: Alias templates?
|
// FIXME: Alias templates?
|
||||||
|
return D;
|
||||||
|
}
|
||||||
|
} // unnamed namespace
|
||||||
|
|
||||||
|
const RawComment *ASTContext::getRawCommentForAnyRedecl(const Decl *D) const {
|
||||||
|
D = adjustDeclToTemplate(D);
|
||||||
|
|
||||||
// Check whether we have cached a comment for this declaration already.
|
// Check whether we have cached a comment for this declaration already.
|
||||||
{
|
{
|
||||||
|
@ -259,11 +270,20 @@ const RawComment *ASTContext::getRawCommentForAnyRedecl(const Decl *D) const {
|
||||||
}
|
}
|
||||||
|
|
||||||
comments::FullComment *ASTContext::getCommentForDecl(const Decl *D) const {
|
comments::FullComment *ASTContext::getCommentForDecl(const Decl *D) const {
|
||||||
|
D = adjustDeclToTemplate(D);
|
||||||
|
const Decl *Canonical = D->getCanonicalDecl();
|
||||||
|
llvm::DenseMap<const Decl *, comments::FullComment *>::iterator Pos =
|
||||||
|
ParsedComments.find(Canonical);
|
||||||
|
if (Pos != ParsedComments.end())
|
||||||
|
return Pos->second;
|
||||||
|
|
||||||
const RawComment *RC = getRawCommentForAnyRedecl(D);
|
const RawComment *RC = getRawCommentForAnyRedecl(D);
|
||||||
if (!RC)
|
if (!RC)
|
||||||
return NULL;
|
return NULL;
|
||||||
|
|
||||||
return RC->getParsed(*this);
|
comments::FullComment *FC = RC->parse(*this, D);
|
||||||
|
ParsedComments[Canonical] = FC;
|
||||||
|
return FC;
|
||||||
}
|
}
|
||||||
|
|
||||||
void
|
void
|
||||||
|
|
|
@ -65,7 +65,7 @@ bool mergedCommentIsTrailingComment(StringRef Comment) {
|
||||||
RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR,
|
RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR,
|
||||||
bool Merged) :
|
bool Merged) :
|
||||||
Range(SR), RawTextValid(false), BriefTextValid(false),
|
Range(SR), RawTextValid(false), BriefTextValid(false),
|
||||||
IsAlmostTrailingComment(false),
|
IsAttached(false), IsAlmostTrailingComment(false),
|
||||||
BeginLineValid(false), EndLineValid(false) {
|
BeginLineValid(false), EndLineValid(false) {
|
||||||
// Extract raw comment text, if possible.
|
// Extract raw comment text, if possible.
|
||||||
if (SR.getBegin() == SR.getEnd() || getRawText(SourceMgr).empty()) {
|
if (SR.getBegin() == SR.getEnd() || getRawText(SourceMgr).empty()) {
|
||||||
|
@ -87,16 +87,6 @@ RawComment::RawComment(const SourceManager &SourceMgr, SourceRange SR,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const Decl *RawComment::getDecl() const {
|
|
||||||
if (DeclOrParsedComment.isNull())
|
|
||||||
return NULL;
|
|
||||||
|
|
||||||
if (const Decl *D = DeclOrParsedComment.dyn_cast<const Decl *>())
|
|
||||||
return D;
|
|
||||||
|
|
||||||
return DeclOrParsedComment.get<comments::FullComment *>()->getDecl();
|
|
||||||
}
|
|
||||||
|
|
||||||
unsigned RawComment::getBeginLine(const SourceManager &SM) const {
|
unsigned RawComment::getBeginLine(const SourceManager &SM) const {
|
||||||
if (BeginLineValid)
|
if (BeginLineValid)
|
||||||
return BeginLine;
|
return BeginLine;
|
||||||
|
@ -169,7 +159,8 @@ const char *RawComment::extractBriefText(const ASTContext &Context) const {
|
||||||
return BriefTextPtr;
|
return BriefTextPtr;
|
||||||
}
|
}
|
||||||
|
|
||||||
comments::FullComment *RawComment::parse(const ASTContext &Context) const {
|
comments::FullComment *RawComment::parse(const ASTContext &Context,
|
||||||
|
const Decl *D) const {
|
||||||
// Make sure that RawText is valid.
|
// Make sure that RawText is valid.
|
||||||
getRawText(Context.getSourceManager());
|
getRawText(Context.getSourceManager());
|
||||||
|
|
||||||
|
@ -179,13 +170,11 @@ comments::FullComment *RawComment::parse(const ASTContext &Context) const {
|
||||||
RawText.begin(), RawText.end());
|
RawText.begin(), RawText.end());
|
||||||
comments::Sema S(Context.getAllocator(), Context.getSourceManager(),
|
comments::Sema S(Context.getAllocator(), Context.getSourceManager(),
|
||||||
Context.getDiagnostics(), Traits);
|
Context.getDiagnostics(), Traits);
|
||||||
S.setDecl(getDecl());
|
S.setDecl(D);
|
||||||
comments::Parser P(L, S, Context.getAllocator(), Context.getSourceManager(),
|
comments::Parser P(L, S, Context.getAllocator(), Context.getSourceManager(),
|
||||||
Context.getDiagnostics(), Traits);
|
Context.getDiagnostics(), Traits);
|
||||||
|
|
||||||
comments::FullComment *FC = P.parseFullComment();
|
return P.parseFullComment();
|
||||||
DeclOrParsedComment = FC;
|
|
||||||
return FC;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
namespace {
|
namespace {
|
||||||
|
|
|
@ -7623,7 +7623,9 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D) {
|
||||||
<< FD->getName() << "dllimport";
|
<< FD->getName() << "dllimport";
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
ActOnDocumentableDecl(FD);
|
// We want to attach documentation to original Decl (which might be
|
||||||
|
// a function template).
|
||||||
|
ActOnDocumentableDecl(D);
|
||||||
return FD;
|
return FD;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -624,13 +624,9 @@ class test_attach37 {
|
||||||
/// \brief\author Aaa
|
/// \brief\author Aaa
|
||||||
/// \tparam T Aaa
|
/// \tparam T Aaa
|
||||||
void test_attach38(int aaa, int bbb);
|
void test_attach38(int aaa, int bbb);
|
||||||
};
|
|
||||||
|
|
||||||
// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
|
void test_attach39(int aaa, int bbb);
|
||||||
/// \brief\author Aaa
|
};
|
||||||
/// \tparam T Aaa
|
|
||||||
template<typename T>
|
|
||||||
void test_attach37<T>::test_attach38(int aaa, int bbb) {}
|
|
||||||
|
|
||||||
// expected-warning@+2 {{empty paragraph passed to '\brief' command}}
|
// expected-warning@+2 {{empty paragraph passed to '\brief' command}}
|
||||||
// expected-warning@+2 {{template parameter 'T' not found in the template declaration}}
|
// expected-warning@+2 {{template parameter 'T' not found in the template declaration}}
|
||||||
|
@ -639,6 +635,13 @@ void test_attach37<T>::test_attach38(int aaa, int bbb) {}
|
||||||
template<>
|
template<>
|
||||||
void test_attach37<int>::test_attach38(int aaa, int bbb) {}
|
void test_attach37<int>::test_attach38(int aaa, int bbb) {}
|
||||||
|
|
||||||
|
// expected-warning@+1 {{empty paragraph passed to '\brief' command}}
|
||||||
|
/// \brief\author Aaa
|
||||||
|
/// \tparam T Aaa
|
||||||
|
template<typename T>
|
||||||
|
void test_attach37<T>::test_attach39(int aaa, int bbb) {}
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
// PR13411, reduced. We used to crash on this.
|
// PR13411, reduced. We used to crash on this.
|
||||||
/**
|
/**
|
||||||
|
@ -652,7 +655,7 @@ void test_nocrash1(int);
|
||||||
/// \param\brief
|
/// \param\brief
|
||||||
void test_nocrash2(int);
|
void test_nocrash2(int);
|
||||||
|
|
||||||
// PR13593
|
// PR13593, example 1 and 2
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Bla.
|
* Bla.
|
||||||
|
@ -668,3 +671,30 @@ template <typename>
|
||||||
void test_nocrash3()
|
void test_nocrash3()
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// PR13593, example 3
|
||||||
|
|
||||||
|
/**
|
||||||
|
* aaa
|
||||||
|
*/
|
||||||
|
template <typename T>
|
||||||
|
inline T test_nocrash5(T a1)
|
||||||
|
{
|
||||||
|
return a1;
|
||||||
|
}
|
||||||
|
|
||||||
|
///
|
||||||
|
//,
|
||||||
|
|
||||||
|
inline void test_nocrash6()
|
||||||
|
{
|
||||||
|
test_nocrash5(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
// We used to crash on this.
|
||||||
|
|
||||||
|
/*!
|
||||||
|
Blah.
|
||||||
|
*/
|
||||||
|
typedef const struct test_nocrash7 * test_nocrash8;
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue