[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

2019-04-12 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:728
 
-  class RawCommentAndCacheFlags {
+  class CommentAndOrigin {
   public:

`RawCommentAndOrigin`?



Comment at: clang/include/clang/AST/ASTContext.h:751
 
-  /// Mapping from declarations to comments attached to any
+  /// Mapping from canonical declarations to comments attached to any
   /// redeclaration.

Please remove "canonical", it is not correct according to the implementation.

Please also update the patch description that currently says "I assume we can 
cache comments for canonical declarations only, not for every redeclaration."



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

jkorous wrote:
> gribozavr wrote:
> > jkorous wrote:
> > > arphaman wrote:
> > > > Why are we now checking for the canonical declaration instead of `D` as 
> > > > before?
> > > Before this we were caching comment for every redeclaration separately 
> > > but for every redeclaration the comment was the same.
> > > 
> > > As I understand it - for a given declaration we found the first comment 
> > > in the redeclaration chain and then saved it to the cache for every 
> > > redeclaration (the same comment).
> > > Later, during lookup, we iterated the redeclaration chain again and did a 
> > > lookup for every redeclaration (see L392 in the original implementation).
> > > 
> > > Unless I missed something, it's suboptimal in both memory consumption and 
> > > runtime overhead.
> > > 
> > > That's the reason why I want to cache the comment for the redeclaration 
> > > group as a whole. The only thing I am not entirely sure about is what key 
> > > to use to represent the whole redeclaration chain - maybe the first 
> > > declaration in the redeclaration chain would be better then the canonical 
> > > declaration...
> > > 
> > > WDYT?
> > > As I understand it - for a given declaration we found the first comment 
> > > in the redeclaration chain and then saved it to the cache for every 
> > > redeclaration (the same comment).
> > 
> > Only if there was only one comment in the redeclaration chain.  If a 
> > redeclaration had a different comment, this function would return that 
> > comment.
> I see. I made a wrong assumption - that for any two redeclarations the 
> redeclaration chain always starts from the same declaration.
I still don't think this patch is a no-op.

Here is the intended behavior:

```
/// First
void foo(); // (1), canonical decl

void foo(); // (2)

/// Third
void foo() {} // (3)

void foo(); // (4)
```

getRawCommentForAnyRedecl(1) => "First"
getRawCommentForAnyRedecl(2) => "First"
getRawCommentForAnyRedecl(3) => "Third"
getRawCommentForAnyRedecl(4) => "First"




Comment at: clang/lib/AST/ASTContext.cpp:373
 
-  // Check whether we have cached a comment for this declaration already.
-  {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
+  auto CacheLookup = [&](const Decl *SpecificD) -> const RawComment * {
+llvm::DenseMap::iterator Pos =

`GetCommentFromCache`?



Comment at: clang/lib/AST/ASTContext.cpp:405
+  // Check whether we have cached a comment for this specific declaration.
+  if (auto * CachedComment = CacheLookup(D))
+return CachedComment;

No space after `*`.



Comment at: clang/lib/AST/ASTContext.cpp:415
+
+  // In case this is the canonical Decl we're done.
+  auto CanonicalD = D->getCanonicalDecl();

I don't understand the comment -- I read it as "if we got here, and the decl 
was canonical, and we haven't found anything already, we should return 
nullptr".  The code does something different.

Also, just to be clear, here is the intended behavior:

```
void foo(); // (1), canonical decl

/// Second
void foo(); // (2)
```

getRawCommentForAnyRedecl(1) => "Second"
getRawCommentForAnyRedecl(2) => "Second"




Comment at: clang/lib/AST/ASTContext.cpp:418
+  if (!CanonicalD)
+return nullptr;
 

When is it possible to not have a canonical decl?



Comment at: clang/lib/AST/ASTContext.cpp:421
+  // Check whether we have cached a comment for canonical declaration of this 
declaration.
+  if (auto * CachedComment = CacheLookup(CanonicalD))
+return CachedComment;

No space after `*`.



Comment at: clang/lib/AST/ASTContext.cpp:425
+  // We don't have comment for neither D, nor it's canonical declaration in 
cache and D doesn't have any comment attached to itself.
+  // Search for any comment attached to redeclarations of D and cache it for 
canonical declaration of D.
   for (auto I : D->redecls()) {

80 columns.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60432/new/


[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

2019-04-10 Thread Jan Korous via Phabricator via cfe-commits
jkorous updated this revision to Diff 194609.
jkorous added a comment.

rename RawCommentAndCacheFlags -> CommentAndOrigin


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60432/new/

https://reviews.llvm.org/D60432

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp

Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -370,70 +370,69 @@
 const Decl **OriginalDecl) const {
   D = adjustDeclToTemplate(D);
 
-  // Check whether we have cached a comment for this declaration already.
-  {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
+  auto CacheLookup = [&](const Decl *SpecificD) -> const RawComment * {
+llvm::DenseMap::iterator Pos =
+RedeclComments.find(SpecificD);
 if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-if (OriginalDecl)
-  *OriginalDecl = Raw.getOriginalDecl();
-return Raw.getRaw();
-  }
+  const CommentAndOrigin  = Pos->second;
+  if (OriginalDecl)
+*OriginalDecl = CachedComment.getOriginalDecl();
+  return CachedComment.getRaw();
 }
-  }
+return nullptr;
+  };
 
-  // Search for comments attached to declarations in the redeclaration chain.
-  const RawComment *RC = nullptr;
-  const Decl *OriginalDeclForRC = nullptr;
-  for (auto I : D->redecls()) {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(I);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-RC = Raw.getRaw();
-OriginalDeclForRC = Raw.getOriginalDecl();
-break;
-  }
-} else {
-  RC = getRawCommentForDeclNoCache(I);
-  OriginalDeclForRC = I;
-  RawCommentAndCacheFlags Raw;
-  if (RC) {
-// Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-// https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-Raw.setRaw(RC);
-  } else
-Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-  Raw.setOriginalDecl(I);
-  RedeclComments[I] = Raw;
-  if (RC)
-break;
+  auto GetCommentNoCache = [&](const Decl *SpecificD) -> const llvm::Optional {
+if (const RawComment *RC = getRawCommentForDeclNoCache(SpecificD)) {
+  // If we find a comment, it should be a documentation comment.
+  assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+
+  CommentAndOrigin NewCachedComment;
+  // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
+  // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
+  NewCachedComment.setRaw(RC);
+  NewCachedComment.setOriginalDecl(SpecificD);
+
+  if (OriginalDecl)
+*OriginalDecl = SpecificD;
+
+  return NewCachedComment;
 }
-  }
+return llvm::None;
+  };
 
-  // If we found a comment, it should be a documentation comment.
-  assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  // Check whether we have cached a comment for this specific declaration.
+  if (auto * CachedComment = CacheLookup(D))
+return CachedComment;
 
-  if (OriginalDecl)
-*OriginalDecl = OriginalDeclForRC;
+  // Try to find comment for this specific declaration.
+  if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(D)) {
+const CommentAndOrigin& CommentAndFlags = OptCommentAndFlags.getValue();
+RedeclComments[D] = CommentAndFlags;
+return CommentAndFlags.getRaw();
+  }
+
+  // In case this is the canonical Decl we're done.
+  auto CanonicalD = D->getCanonicalDecl();
+  if (!CanonicalD)
+return nullptr;
 
-  // Update cache for every declaration in the redeclaration chain.
-  RawCommentAndCacheFlags Raw;
-  Raw.setRaw(RC);
-  Raw.setKind(RawCommentAndCacheFlags::FromRedecl);
-  Raw.setOriginalDecl(OriginalDeclForRC);
+  // Check whether we have cached a comment for canonical declaration of this declaration.
+  if (auto * CachedComment = CacheLookup(CanonicalD))
+return CachedComment;
 
+  // We don't have comment for neither D, nor it's canonical declaration in cache and D doesn't have any comment attached to itself.
+  // Search for any comment attached to redeclarations of D and cache it for canonical declaration of D.
   for (auto I : D->redecls()) {
-RawCommentAndCacheFlags  = RedeclComments[I];
-if (R.getKind() == RawCommentAndCacheFlags::NoCommentInDecl)
-  R = Raw;
+if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(I)) {
+  const CommentAndOrigin& CommentAndFlags = OptCommentAndFlags.getValue();
+  RedeclComments[CanonicalD] = 

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

2019-04-10 Thread Jan Korous via Phabricator via cfe-commits
jkorous updated this revision to Diff 194604.
jkorous added a comment.

Second attempt on reducing the cache size and number of operations.

I try in this order

1. cache lookup for the specific provided declaration
2. try to find comment attached to the specific provided declaration without 
using cache (and cache it using the specific declaration as a key)
3. cache lookup using the canonical declaration (which would return comment 
from any redeclaration - see the next step)
4. try to find comment attached to any redeclaration (and cache it using the 
canonical declaration as a key)
5. give up, return nullptr


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60432/new/

https://reviews.llvm.org/D60432

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp

Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -370,70 +370,69 @@
 const Decl **OriginalDecl) const {
   D = adjustDeclToTemplate(D);
 
-  // Check whether we have cached a comment for this declaration already.
-  {
+  auto CacheLookup = [&](const Decl *SpecificD) -> const RawComment * {
 llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
+RedeclComments.find(SpecificD);
 if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-if (OriginalDecl)
-  *OriginalDecl = Raw.getOriginalDecl();
-return Raw.getRaw();
-  }
+  const RawCommentAndCacheFlags  = Pos->second;
+  if (OriginalDecl)
+*OriginalDecl = CachedComment.getOriginalDecl();
+  return CachedComment.getRaw();
 }
-  }
+return nullptr;
+  };
 
-  // Search for comments attached to declarations in the redeclaration chain.
-  const RawComment *RC = nullptr;
-  const Decl *OriginalDeclForRC = nullptr;
-  for (auto I : D->redecls()) {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(I);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-RC = Raw.getRaw();
-OriginalDeclForRC = Raw.getOriginalDecl();
-break;
-  }
-} else {
-  RC = getRawCommentForDeclNoCache(I);
-  OriginalDeclForRC = I;
-  RawCommentAndCacheFlags Raw;
-  if (RC) {
-// Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-// https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-Raw.setRaw(RC);
-  } else
-Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-  Raw.setOriginalDecl(I);
-  RedeclComments[I] = Raw;
-  if (RC)
-break;
+  auto GetCommentNoCache = [&](const Decl *SpecificD) -> const llvm::Optional {
+if (const RawComment *RC = getRawCommentForDeclNoCache(SpecificD)) {
+  // If we find a comment, it should be a documentation comment.
+  assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+
+  RawCommentAndCacheFlags NewCachedComment;
+  // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
+  // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
+  NewCachedComment.setRaw(RC);
+  NewCachedComment.setOriginalDecl(SpecificD);
+
+  if (OriginalDecl)
+*OriginalDecl = SpecificD;
+
+  return NewCachedComment;
 }
-  }
+return llvm::None;
+  };
 
-  // If we found a comment, it should be a documentation comment.
-  assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  // Check whether we have cached a comment for this specific declaration.
+  if (auto * CachedComment = CacheLookup(D))
+return CachedComment;
 
-  if (OriginalDecl)
-*OriginalDecl = OriginalDeclForRC;
+  // Try to find comment for this specific declaration.
+  if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(D)) {
+const RawCommentAndCacheFlags& CommentAndFlags = OptCommentAndFlags.getValue();
+RedeclComments[D] = CommentAndFlags;
+return CommentAndFlags.getRaw();
+  }
+
+  // In case this is the canonical Decl we're done.
+  auto CanonicalD = D->getCanonicalDecl();
+  if (!CanonicalD)
+return nullptr;
 
-  // Update cache for every declaration in the redeclaration chain.
-  RawCommentAndCacheFlags Raw;
-  Raw.setRaw(RC);
-  Raw.setKind(RawCommentAndCacheFlags::FromRedecl);
-  Raw.setOriginalDecl(OriginalDeclForRC);
+  // Check whether we have cached a comment for canonical declaration of this declaration.
+  if (auto * CachedComment = CacheLookup(CanonicalD))
+return CachedComment;
 
+  // We don't have comment for neither D, nor it's canonical declaration in cache and D doesn't have any comment 

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

2019-04-10 Thread Jan Korous via Phabricator via cfe-commits
jkorous marked an inline comment as done.
jkorous added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

gribozavr wrote:
> jkorous wrote:
> > arphaman wrote:
> > > Why are we now checking for the canonical declaration instead of `D` as 
> > > before?
> > Before this we were caching comment for every redeclaration separately but 
> > for every redeclaration the comment was the same.
> > 
> > As I understand it - for a given declaration we found the first comment in 
> > the redeclaration chain and then saved it to the cache for every 
> > redeclaration (the same comment).
> > Later, during lookup, we iterated the redeclaration chain again and did a 
> > lookup for every redeclaration (see L392 in the original implementation).
> > 
> > Unless I missed something, it's suboptimal in both memory consumption and 
> > runtime overhead.
> > 
> > That's the reason why I want to cache the comment for the redeclaration 
> > group as a whole. The only thing I am not entirely sure about is what key 
> > to use to represent the whole redeclaration chain - maybe the first 
> > declaration in the redeclaration chain would be better then the canonical 
> > declaration...
> > 
> > WDYT?
> > As I understand it - for a given declaration we found the first comment in 
> > the redeclaration chain and then saved it to the cache for every 
> > redeclaration (the same comment).
> 
> Only if there was only one comment in the redeclaration chain.  If a 
> redeclaration had a different comment, this function would return that 
> comment.
I see. I made a wrong assumption - that for any two redeclarations the 
redeclaration chain always starts from the same declaration.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60432/new/

https://reviews.llvm.org/D60432



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

2019-04-10 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:756
   /// lazily.
   mutable llvm::DenseMap RedeclComments;
 

The name of this variable (and `RawCommentAndCacheFlags`) don't make sense 
after the change.



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

jkorous wrote:
> arphaman wrote:
> > Why are we now checking for the canonical declaration instead of `D` as 
> > before?
> Before this we were caching comment for every redeclaration separately but 
> for every redeclaration the comment was the same.
> 
> As I understand it - for a given declaration we found the first comment in 
> the redeclaration chain and then saved it to the cache for every 
> redeclaration (the same comment).
> Later, during lookup, we iterated the redeclaration chain again and did a 
> lookup for every redeclaration (see L392 in the original implementation).
> 
> Unless I missed something, it's suboptimal in both memory consumption and 
> runtime overhead.
> 
> That's the reason why I want to cache the comment for the redeclaration group 
> as a whole. The only thing I am not entirely sure about is what key to use to 
> represent the whole redeclaration chain - maybe the first declaration in the 
> redeclaration chain would be better then the canonical declaration...
> 
> WDYT?
> As I understand it - for a given declaration we found the first comment in 
> the redeclaration chain and then saved it to the cache for every 
> redeclaration (the same comment).

Only if there was only one comment in the redeclaration chain.  If a 
redeclaration had a different comment, this function would return that comment.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60432/new/

https://reviews.llvm.org/D60432



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

2019-04-09 Thread Jan Korous via Phabricator via cfe-commits
jkorous marked an inline comment as done.
jkorous added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

arphaman wrote:
> Why are we now checking for the canonical declaration instead of `D` as 
> before?
Before this we were caching comment for every redeclaration separately but for 
every redeclaration the comment was the same.

As I understand it - for a given declaration we found the first comment in the 
redeclaration chain and then saved it to the cache for every redeclaration (the 
same comment).
Later, during lookup, we iterated the redeclaration chain again and did a 
lookup for every redeclaration (see L392 in the original implementation).

Unless I missed something, it's suboptimal in both memory consumption and 
runtime overhead.

That's the reason why I want to cache the comment for the redeclaration group 
as a whole. The only thing I am not entirely sure about is what key to use to 
represent the whole redeclaration chain - maybe the first declaration in the 
redeclaration chain would be better then the canonical declaration...

WDYT?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60432/new/

https://reviews.llvm.org/D60432



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

2019-04-09 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

Why are we now checking for the canonical declaration instead of `D` as before?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60432/new/

https://reviews.llvm.org/D60432



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

2019-04-08 Thread Jan Korous via Phabricator via cfe-commits
jkorous created this revision.
jkorous added reviewers: gribozavr, arphaman, dexonsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- I assume we can cache comments for canonical declarations only, not for every 
redeclaration.
- Caching that we didn't find comments seems to be unnecessary since we try to 
search for comment again again next time if we find this data in cache.
  - We might implement proper cache invalidation in the future and get back to 
using this information.
- Origin of comment (directly from declaration / from some redeclaration) seems 
to not be used anywhere.

I plan to do some performance testing before committing but like to have some 
feedback first.

BTW there's another cache for comments in the `ASTContext` - `ParsedComments`. 
We could try to experiment with different caching approaches to see how it 
affects performance - maybe caching `mapping from canonical declarations to raw 
comments` and separately caching `mapping from raw comments to full comments`.


Repository:
  rC Clang

https://reviews.llvm.org/D60432

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp

Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -369,71 +369,43 @@
 const Decl *D,
 const Decl **OriginalDecl) const {
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 
   // Check whether we have cached a comment for this declaration already.
   {
 llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
+RedeclComments.find(CanonicalDecl);
 if (Pos != RedeclComments.end()) {
   const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-if (OriginalDecl)
-  *OriginalDecl = Raw.getOriginalDecl();
-return Raw.getRaw();
-  }
+  if (OriginalDecl)
+*OriginalDecl = Raw.getOriginalDecl();
+  return Raw.getRaw();
 }
   }
 
-  // Search for comments attached to declarations in the redeclaration chain.
-  const RawComment *RC = nullptr;
-  const Decl *OriginalDeclForRC = nullptr;
+  // We don't have comment for D in cache - search for any comment attached to
+  // declarations in the redeclaration chain.
   for (auto I : D->redecls()) {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(I);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-RC = Raw.getRaw();
-OriginalDeclForRC = Raw.getOriginalDecl();
-break;
-  }
-} else {
-  RC = getRawCommentForDeclNoCache(I);
-  OriginalDeclForRC = I;
-  RawCommentAndCacheFlags Raw;
-  if (RC) {
-// Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-// https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-Raw.setRaw(RC);
-  } else
-Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-  Raw.setOriginalDecl(I);
-  RedeclComments[I] = Raw;
-  if (RC)
-break;
-}
-  }
-
-  // If we found a comment, it should be a documentation comment.
-  assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+if (const RawComment *RC = getRawCommentForDeclNoCache(I)) {
+  // If we find a comment, it should be a documentation comment.
+  assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
 
-  if (OriginalDecl)
-*OriginalDecl = OriginalDeclForRC;
+  if (OriginalDecl)
+*OriginalDecl = I;
 
-  // Update cache for every declaration in the redeclaration chain.
-  RawCommentAndCacheFlags Raw;
-  Raw.setRaw(RC);
-  Raw.setKind(RawCommentAndCacheFlags::FromRedecl);
-  Raw.setOriginalDecl(OriginalDeclForRC);
+  RawCommentAndCacheFlags NewCachedComment;
+  // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
+  // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
+  NewCachedComment.setRaw(RC);
+  NewCachedComment.setOriginalDecl(I);
+  RedeclComments[CanonicalDecl] = NewCachedComment;
 
-  for (auto I : D->redecls()) {
-RawCommentAndCacheFlags  = RedeclComments[I];
-if (R.getKind() == RawCommentAndCacheFlags::NoCommentInDecl)
-  R = Raw;
+  return RC;
+}
   }
 
-  return RC;
+  // We didn't find any comment attached to any redeclaration of D.
+  return nullptr;
 }
 
 static void addRedeclaredMethods(const ObjCMethodDecl *ObjCMethod,
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++