This revision was automatically updated to reflect the committed changes.
Closed by commit rL320471: [SemaCodeComplete] Allow passing out scope
specifiers in qualified-id… (authored by ioeric, committed by ).
Repository:
rL LLVM
https://reviews.llvm.org/D40563
Files:
ioeric added a comment.
Fyi, you could find use of the new API in https://reviews.llvm.org/D40548
I'd like to land this patch if there is no objection.
https://reviews.llvm.org/D40563
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D40563#940536, @ioeric wrote:
> In https://reviews.llvm.org/D40563#939964, @arphaman wrote:
>
> > If nothing uses `getCXXScopeSpecifier` right now we can't
ioeric added a comment.
In https://reviews.llvm.org/D40563#939964, @arphaman wrote:
> If nothing uses `getCXXScopeSpecifier` right now we can't really test it with
> a clang or c-index-test regression test. A completion unit test could work
> here. I don't think we actually have existing
ilya-biryukov added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4609
+ if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
ioeric wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > Why do we alter
arphaman added a comment.
If nothing uses `getCXXScopeSpecifier` right now we can't really test it with a
clang or c-index-test regression test. A completion unit test could work here.
I don't think we actually have existing completion unit tests though, so you
would have to create one from
ioeric added a comment.
In https://reviews.llvm.org/D40563#939555, @arphaman wrote:
> Could you please add a test?
Any tip on how this should be tested? I couldn't find any existing unit test
for either SemaCodeComplete or code completion context (under
`clang/unittests`). It might be
arphaman added a comment.
Could you please add a test?
https://reviews.llvm.org/D40563
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ioeric updated this revision to Diff 124739.
ioeric marked 4 inline comments as done.
ioeric added a comment.
- Address review comments.
https://reviews.llvm.org/D40563
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/SemaCodeComplete.cpp
Index: lib/Sema/SemaCodeComplete.cpp
ioeric added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4609
+ if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Why do we alter this code path?
> Maybe we
ilya-biryukov added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4609
+ if (SS.isInvalid()) {
+CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
ilya-biryukov wrote:
> Why do we alter this code path?
Maybe we should add a test or
ilya-biryukov added inline comments.
Comment at: include/clang/Sema/CodeCompleteConsumer.h:284
+ llvm::Optional ScopeSpecifier;
+
Maybe add a brief comment for consistency with other decls?
Comment at: lib/Sema/SemaCodeComplete.cpp:4609
+
ioeric created this revision.
https://reviews.llvm.org/D40563
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/SemaCodeComplete.cpp
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++
13 matches
Mail list logo