This revision was automatically updated to reflect the committed changes.
Closed by commit rC349349: [ASTImporter] Fix redecl chain of classes and class
templates (authored by martong, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53655?vs=175641=178450#toc
Repository:
martong added a comment.
@rsmith Please raise any objections until Dec 14 (or if this deadline is too
strict). I'd like to commit this next week latest so it can get in still this
year.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53655/new/
martong added a comment.
@rsmith ping
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53655/new/
https://reviews.llvm.org/D53655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
martong added a comment.
@aaron.ballman Thanks for your review. I have updated the patch to remove
`containsInVector`.
> This seems mostly reasonable to me, but @rsmith is more well-versed in this
> area and may have further comments. You should give him a few days to chime
> in before
martong updated this revision to Diff 175641.
martong marked 2 inline comments as done.
martong added a comment.
- Remove containsInVector
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53655/new/
https://reviews.llvm.org/D53655
Files:
martong marked 4 inline comments as done.
martong added inline comments.
Comment at: include/clang/AST/DeclContextInternals.h:125-129
+ bool containsInVector(const NamedDecl *D) {
+assert(getAsVector() && "Must have a vector at this point");
+DeclsTy = *getAsVector();
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This seems mostly reasonable to me, but @rsmith is more well-versed in this
area and may have further comments. You should give him a few days to chime in
before committing.
Comment at:
martong updated this revision to Diff 175482.
martong marked 5 inline comments as done.
martong added a comment.
- Address several minor review comments
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53655/new/
https://reviews.llvm.org/D53655
Files:
martong marked 20 inline comments as done.
martong added inline comments.
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1169
+/// matches 'a'.
+extern const internal::VariadicDynCastAllOfMatcher
+indirectFieldDecl;
aaron.ballman wrote:
> Be sure to
aaron.ballman added inline comments.
Comment at: include/clang/AST/DeclContextInternals.h:128
+DeclsTy = *getAsVector();
+DeclsTy::iterator I = std::find(Vec.begin(), Vec.end(), D);
+return I != Vec.end();
a_sidorin wrote:
> `auto I =
a_sidorin added subscribers: aaron.ballman, rsmith.
a_sidorin added a comment.
Please note that changes in AST lib will require AST code owners' approval.
@rsmith, @aaron.ballman could you please take a look?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
a_sidorin added a comment.
Hi Gabor,
The change looks mostly Ok but there are some comments inline.
Comment at: include/clang/AST/DeclContextInternals.h:128
+DeclsTy = *getAsVector();
+DeclsTy::iterator I = std::find(Vec.begin(), Vec.end(), D);
+return I !=
martong added a comment.
This will break LLDB, unless https://reviews.llvm.org/D54863 is applied.
@shafik Could you please take a look on https://reviews.llvm.org/D54863 ?
Repository:
rC Clang
https://reviews.llvm.org/D53655
___
cfe-commits
martong updated this revision to Diff 175127.
martong added a comment.
- Use MostRecentDecl when setting PrevDecl
Repository:
rC Clang
https://reviews.llvm.org/D53655
Files:
include/clang/AST/DeclContextInternals.h
include/clang/ASTMatchers/ASTMatchers.h
lib/AST/ASTImporter.cpp
balazske accepted this revision.
balazske added a comment.
This revision is now accepted and ready to land.
Do not forget that there is a fix the to use getMostRecentDecl in
ASTImporter.cpp line 2666 here.
Repository:
rC Clang
https://reviews.llvm.org/D53655
martong added a comment.
> I see that now everything is reverted, the "good" things too (change to
> indirectFieldDecl and a line split)?
Yes, you are completely right, sorry for this mess. I just have updated so the
good things remain.
Repository:
rC Clang
https://reviews.llvm.org/D53655
martong updated this revision to Diff 175091.
martong marked an inline comment as done.
martong added a comment.
- Keep the good changes and use the name 'containsInVector'
Repository:
rC Clang
https://reviews.llvm.org/D53655
Files:
include/clang/AST/DeclContextInternals.h
balazske added a comment.
I see that now everything is reverted, the "good" things too (change to
indirectFieldDecl and a line split)?
Repository:
rC Clang
https://reviews.llvm.org/D53655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
martong marked 3 inline comments as done.
martong added inline comments.
Comment at: lib/AST/DeclBase.cpp:1469
assert(Pos != Map->end() && "no lookup entry for decl");
-if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND)
+// Remove the decl
martong updated this revision to Diff 175058.
martong added a comment.
- Revert "Minor style changes and rename a function"
Repository:
rC Clang
https://reviews.llvm.org/D53655
Files:
include/clang/AST/DeclContextInternals.h
include/clang/ASTMatchers/ASTMatchers.h
Szelethus added inline comments.
Comment at: lib/AST/DeclBase.cpp:1469
assert(Pos != Map->end() && "no lookup entry for decl");
-if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND)
+// Remove the decl only if it is contained.
+if
balazske added inline comments.
Comment at: lib/AST/DeclBase.cpp:1469
assert(Pos != Map->end() && "no lookup entry for decl");
-if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND)
+// Remove the decl only if it is contained.
+if
martong updated this revision to Diff 174893.
martong marked 4 inline comments as done.
martong added a comment.
Herald added a reviewer: shafik.
Herald added a subscriber: gamesh411.
- Minor style changes and rename a function
Repository:
rC Clang
https://reviews.llvm.org/D53655
Files:
martong added inline comments.
Comment at: lib/AST/DeclBase.cpp:1469
assert(Pos != Map->end() && "no lookup entry for decl");
-if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND)
+// Remove the decl only if it is contained.
+if
balazske added inline comments.
Comment at: lib/AST/ASTImporter.cpp:5054
+if (!ToTemplated->getPreviousDecl()) {
+ auto *PrevTemplated =
FoundByLookup->getTemplatedDecl()->getMostRecentDecl();
+ if (ToTemplated != PrevTemplated)
This is a long
Szelethus added a comment.
I can't add anything meaningful to the conversation, but I spotted some nits,
so here they are.
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1150
+/// \endcode
+/// fieldDecl()
+/// matches 'a'.
Did you mean to write
martong created this revision.
martong added reviewers: a_sidorin, balazske.
Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: a.sidorin.
The crux of the issue that is being fixed is that lookup could not find
previous
27 matches
Mail list logo
