This revision was automatically updated to reflect the committed changes.
Closed by commit rL333272: Optionally add code completion results for arrow
instead of dot (authored by yvvan, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D41
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
Thanks for addressing the NITs. LGTM!
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:
yvvan updated this revision to Diff 148553.
yvvan added a comment.
NIT-s addressed
https://reviews.llvm.org/D41537
Files:
include/clang/Driver/CC1Options.td
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Sema/CodeCompleteOptions.h
include/clang/Sema/Sema.h
lib/Frontend/ASTUni
yvvan marked an inline comment as done.
yvvan added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4148
+
+ CompletionSucceded |= DoCompletion(Base, IsArrow, None);
+
yvvan wrote:
> ilya-biryukov wrote:
> > NIT: maybe swap the two cases to do the non
yvvan marked 4 inline comments as done.
yvvan added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4148
+
+ CompletionSucceded |= DoCompletion(Base, IsArrow, None);
+
ilya-biryukov wrote:
> NIT: maybe swap the two cases to do the non-fixit ones first
ilya-biryukov added a comment.
I have only a few nits left, otherwise looks good. Thanks for making this
change! Really excited for it to get in finally!
Comment at: include/clang/Driver/CC1Options.td:449
+def code_completion_with_fixits : Flag<["-"], "code-completion-with-fix
yvvan updated this revision to Diff 148164.
yvvan marked 4 inline comments as done.
https://reviews.llvm.org/D41537
Files:
include/clang/Driver/CC1Options.td
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Sema/CodeCompleteOptions.h
include/clang/Sema/Sema.h
lib/Frontend/ASTUnit
yvvan added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4109
+ if (CodeCompleter->includeFixIts()) {
+const SourceRange OpRange(OpLoc, OpLoc.getLocWithOffset(IsArrow ? 2 : 1));
+CompletionSucceded =
ilya-biryukov wrote:
> yvvan wrote:
> >
ilya-biryukov added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4109
+ if (CodeCompleter->includeFixIts()) {
+const SourceRange OpRange(OpLoc, OpLoc.getLocWithOffset(IsArrow ? 2 : 1));
+CompletionSucceded =
yvvan wrote:
> ilya-biryukov wro
yvvan added inline comments.
Comment at: lib/Sema/CodeCompleteConsumer.cpp:559
+const char *Begin =
+SemaRef.SourceMgr.getCharacterData(FixIt.RemoveRange.getBegin());
+const char *End =
ilya-biryukov wrote:
> Unfortunately, that might
ilya-biryukov added a comment.
Sorry, a few more things and we're good to go.
Comment at: include/clang/Driver/CC1Options.td:449
+def code_completion_include_fixits : Flag<["-"],
"code-completion-include-fixits">,
+ HelpText<"Include code completion results which require havi
yvvan updated this revision to Diff 147039.
yvvan marked 4 inline comments as done.
yvvan added a comment.
Append PrintingCodeCompleteConsumer and CompilerInvocation options, add
CodeCompletion tests.
https://reviews.llvm.org/D41537
Files:
include/clang/Driver/CC1Options.td
include/clang/S
yvvan added a comment.
I will add more tests...
Comment at: test/SemaCXX/member-expr.cpp:193
+Cl0* c;
+return c.a; // expected-error {{member reference type 'PR15045::Cl0 *' is
a pointer; did you mean to use '->'?}}
+ }
ilya-biryukov wrote:
> Is this
ilya-biryukov added a comment.
Maybe move the tests back to this revision?
There are tests for code completion that don't depend on libindex or libclang
and use clang -cc1 directly, i.e. `tools/clang/test/CodeComplete`. This would
require adding a flag to clang and patching up `PrintingCodeCompl
yvvan added inline comments.
Comment at: lib/Sema/SemaCodeComplete.cpp:4118
+ : diag::err_typecheck_member_reference_suggestion;
+ Diag(OpLoc, DiagID) << ConvertedBaseType << IsArrow
+ << Base->getSourceRange() << FixIt;
yvvan updated this revision to Diff 146749.
yvvan marked 2 inline comments as done.
yvvan added a comment.
Only C++ part, libclang part has moved to https://reviews.llvm.org/D46862
https://reviews.llvm.org/D41537
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/CodeCompl
ilya-biryukov added inline comments.
Comment at: tools/libclang/CIndexCodeCompletion.cpp:309
+ /// before that result for the corresponding completion item.
+ std::vector> FixItsVector;
};
yvvan wrote:
> ilya-biryukov wrote:
> > Storing `vector>` here makes lo
yvvan added a comment.
In https://reviews.llvm.org/D41537#1097763, @ilya-biryukov wrote:
> I should've suggested splitting the change into two earlier.
Next time I will do that from the beginning :)
Comment at: tools/libclang/CIndexCodeCompletion.cpp:309
+ /// before that r
ilya-biryukov added a comment.
In https://reviews.llvm.org/D41537#1097601, @yvvan wrote:
> I hope this review will be over some day :)
Sorry for keeping it that long.
FWIW, I think we keep jumping back and forth between the clang and libclang
changes here.
I should've suggested splitting the c
yvvan added a comment.
ping
I hope this review won't take forever :)
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yvvan updated this revision to Diff 146289.
yvvan marked 3 inline comments as done.
yvvan added a comment.
Only keep small completion fixit-s in CodeCompletionResults.
Change libclang calls to overcome that and not use CXCompletionString
https://reviews.llvm.org/D41537
Files:
include/clang-c/
yvvan marked 3 inline comments as done.
yvvan added a comment.
I have some failing tests... So I will update the diff a bit later (Friday most
likely)
Comment at: include/clang/Sema/CodeCompleteConsumer.h:564
+
+ /// \brief For this completion result correction is required.
+
ilya-biryukov added inline comments.
Comment at: include/clang-c/Index.h:5237
+/**
+ * \brief FixIts that *must* be applied before inserting the text for the
+ * corresponding completion item. Completion items with non-empty fixits will
This seems too large for a
yvvan updated this revision to Diff 145664.
yvvan added a comment.
Address comments and provide diff with full context
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Sema/C
yvvan added inline comments.
Comment at: include/clang-c/Index.h:5220
+CINDEX_LINKAGE CXString
+clang_getCompletionCorrection(CXCompletionString completion_string,
+ unsigned correction_index,
ilya-biryukov wrote:
> I'm a bit vary abo
ilya-biryukov added a comment.
Sorry for the delay. I really like the direction of this patch!
What's left is defining the semantics of corrections more thoroughly to make
sure we don't have tricky corner cases that users of the API can't deal with.
PS. This patch is still lacking full context o
yvvan added a comment.
Ping
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yvvan added inline comments.
Comment at: include/clang/Sema/CodeCompleteConsumer.h:704
+CXAvailabilityKind Availability,
+const std::vector &Corrections)
: Allocator(Allocator), CCTUInfo(CCTUInfo), Priority(Priority),
---
yvvan updated this revision to Diff 144285.
yvvan marked 4 inline comments as done.
yvvan added a comment.
Minor clean-up according to the last comments
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/CodeComplet
ilya-biryukov added a comment.
Could you upload the patch with full context?
Comment at: include/clang/Sema/CodeCompleteConsumer.h:704
+CXAvailabilityKind Availability,
+const std::vector &Corrections)
: Allocator(Allocator
yvvan updated this revision to Diff 144076.
yvvan added a comment.
Wrapper around FIxItHint is removed, other review comments are addressed .
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/CodeCompleteConsumer.h
yvvan added inline comments.
Comment at: include/clang-c/Index.h:5278
+ /**
+ * \brief Whether to try dot to arrow correction if arrow operator can be
applied.
+ */
ilya-biryukov wrote:
> This implies that "dot to arrow" is the only available correction. Ma
ilya-biryukov added a comment.
Important: please upload the patch with full context diff
Comment at: include/clang-c/Index.h:5278
+ /**
+ * \brief Whether to try dot to arrow correction if arrow operator can be
applied.
+ */
This implies that "dot to arro
yvvan updated this revision to Diff 143572.
yvvan added a comment.
Use vector instead of optional as Ilya suggested
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Sema/Code
yvvan updated this revision to Diff 143566.
yvvan added a comment.
Use wrapped FixItHint to keep corrections.
Can be quite easily changed from Optional to some container if required.
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Basic/DiagnosticSemaKinds.td
ilya-biryukov added a comment.
Sorry for the late response, was on vacation.
Comment at: include/clang/Sema/CodeCompleteConsumer.h:565
+ /// \brief For this completion result correction is required.
+ Optional Corr = None;
+
yvvan wrote:
> yvvan wrote:
> > il
yvvan added inline comments.
Comment at: include/clang/Sema/CodeCompleteConsumer.h:565
+ /// \brief For this completion result correction is required.
+ Optional Corr = None;
+
yvvan wrote:
> ilya-biryukov wrote:
> > Having a string replacement without an actua
yvvan added inline comments.
Comment at: include/clang/Sema/CodeCompleteConsumer.h:565
+ /// \brief For this completion result correction is required.
+ Optional Corr = None;
+
ilya-biryukov wrote:
> Having a string replacement without an actual range to replac
ilya-biryukov added a comment.
Sorry for the delay.
Comment at: include/clang/Sema/CodeCompleteConsumer.h:565
+ /// \brief For this completion result correction is required.
+ Optional Corr = None;
+
Having a string replacement without an actual range to repl
yvvan added a comment.
Ping
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yvvan updated this revision to Diff 138139.
yvvan added a comment.
Return possibly required corrections in the string form
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Se
ilya-biryukov added a comment.
In https://reviews.llvm.org/D41537#1015563, @yvvan wrote:
> Or is your idea is to return the char sequence instead to use this correction
> in some universal way?
Exactly. Editors that implement corrections would pick up any new corrections
automatically, rather
yvvan added a comment.
Or is your idea is to return the char sequence instead to use this correction
in some universal way?
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
yvvan added a comment.
I've already added hints in this patch and it also do not add extra completions
unless the flag IncludeCorrections is set. So this will not force editors to
use corrections.
Did you mean that you think it's good to have extra fixit hints even if this
flag is not set?
h
ilya-biryukov added a comment.
Sorry for not getting back on this earlier.
I wanted to discuss whether returning correction as a enum value is a proper
interface for users of `libclang`.
It seems easy to replace `.` with `->` inside clang, properly handling all the
tricky cases (tokens coming f
yvvan added a comment.
ping
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yvvan added a comment.
ping...
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yvvan updated this revision to Diff 132146.
yvvan added a comment.
Use https://reviews.llvm.org/D42474 code. Add missing parts and tests for
errors and fixits.
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Cod
yvvan marked 4 inline comments as done.
yvvan added a comment.
@ilya-biryukov
Thanks a lot for you comments and for the provided code replacement. I'm
checking now how it works and will continue addressing other comments.
https://reviews.llvm.org/D41537
__
ilya-biryukov added a comment.
I also propose to try correcting in both directions, i.e. here's how completion
completion could work with this feature:
struct X { int foobarbaz; };
void test() {
X var;
X* ptr;
std::unique_ptr uptr;
var->foobar // replace with var.foo
ilya-biryukov added a comment.
Sorry for not taking a look for a long time. I think that would be a very
useful feature to have.
Here are a few comments (also see the inline comments)
- Please add tests
- How would the clients know how to correct dot to arrow? It'd be cool if code
completion re
yvvan added a comment.
Ping!!!
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yvvan added a comment.
One more Ping!
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yvvan updated this revision to Diff 130384.
yvvan added a comment.
Rebased. Applies for current master.
Also ping again...
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Sema/CodeCompleteOptions.h
lib/Frontend/ASTU
yvvan added a comment.
ping
https://reviews.llvm.org/D41537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yvvan updated this revision to Diff 128509.
yvvan added a comment.
Update CIndex minor version, add call to libclang.exports
https://reviews.llvm.org/D41537
Files:
include/clang-c/Index.h
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Sema/CodeCompleteOptions.h
lib/Frontend/AST
yvvan created this revision.
yvvan added reviewers: klimek, bkramer, arphaman, ilya-biryukov, erikjv.
Currently getting such completions requires source correction, reparsing and
calling completion again. And if it shows no results and rollback is required
then it costs one more reparse.
With th
57 matches
Mail list logo
