GNOME Bugzilla – Bug 786700
Code Search for Builder
Last modified: 2017-10-16 08:58:55 UTC
This is for tracking GSOC Project Code Search for GNOME Builder,https://summerofcode.withgoogle.com/projects/#4574885478137856. Continuation of https://bugzilla.gnome.org/show_bug.cgi?id=783764.
Created attachment 358271 [details] [review] code-index: Adding IdeCodeIndexService to code-index
Created attachment 358272 [details] [review] symbols: Added symbols, flags and icons
Created attachment 358273 [details] [review] symbols: Adding IdeCodeIndexer,IdeCodeIndexEntry interfaces
Created attachment 358274 [details] [review] Adding persistent map
Created attachment 358275 [details] [review] code-index: Adding IdeCodeIndexBuilder
Created attachment 358276 [details] [review] code-index: Adding IdeCodeIndexIndex
Created attachment 358278 [details] [review] clang: Implement IdeCodeIndexer
Created attachment 358279 [details] [review] clang: Implement GListModel, IdeCodeIndexEntry
Created attachment 358280 [details] [review] code-index: Implementing IdeSearchResult
Created attachment 358281 [details] [review] code-index: Implmenting search provider
Created attachment 358282 [details] [review] code-index: Implementing symbol resolver
Created attachment 358283 [details] [review] buffer: Use multiple symbol resolvers instead of one.
Created attachment 358284 [details] [review] code-index: Adding plugin files
Created attachment 358285 [details] [review] clang: Adding code indexer
Created attachment 358286 [details] [review] buildsystem: Handle empty file array and cancellation of task
Created attachment 358287 [details] [review] libide: Unload services before destroying RuntimeManager
Created attachment 358288 [details] [review] code-index: Removing some messages and cleaning code
Review of attachment 358271 [details] [review]: Couple leaks to fix. If I manage to get to it today, I might just merge + fixup afterwards, but time will tell. ::: plugins/code-index/ide-code-index-service.c @@ +73,3 @@ + +static void +build_data_free (BuildData *data, No need to have the user_data parameter, it's fine to cast a (GDestroyNotify) to (GFunc) as long as you don't use user_data. @@ +92,3 @@ + gpointer user_data) +{ + g_autoptr(IdeCodeIndexService) self = (IdeCodeIndexService *)user_data; You don't need casts for gpointer/void* in C. Also, this leaks self since it receives a reference when called. Use g_autoptr(IdeCodeIndexService) self = user_data; to fix it. @@ +188,3 @@ + bdata->recursive = recursive; + + source_id = g_timeout_add_seconds (5, (GSourceFunc)ide_code_index_serivce_push, bdata); Use a #define for something like DEFAULT_INDEX_TIMEOUT_SECS or something like that. @@ +381,3 @@ + g_hash_table_insert (self->code_indexers, + g_strdup (languages[i]), + g_object_ref (adapter)); g_steal_pointer (&adapter) @@ +454,3 @@ + IdeCodeIndexService *self = (IdeCodeIndexService *)service; + + if (self->cancellable != NULL) No need for the if check, g_cancellable_cancel() accepts NULL @@ +497,3 @@ +} + +// static IdeCodeIndexer * It would be nice to remove the unused code
Review of attachment 358272 [details] [review]: Cool, LGTM
Review of attachment 358273 [details] [review]: Looks good. If you feel like it, add "Since: 3.26" to the gtk-doc blocks since this is API that can be consumed by plugins. ::: libide/symbols/ide-code-index-entry.c @@ +27,3 @@ +{ +} + We could use some documentation on these interface methods, since "USR" isn't a well known word outside of those that have used libclang. ::: libide/symbols/ide-code-index-entry.h @@ +44,3 @@ +}; + +/* make this const */ I think you can remove this comment, because in general we don't want to use "const strings" from interfaces as it makes things extra difficult on language bindings (which can leak strings in those scenarios). It's why most of our other interfaces are gchar*. ::: libide/symbols/ide-code-indexer.c @@ +96,3 @@ + IdeCodeIndexerInterface *iface; + + g_return_val_if_fail (IDE_IS_CODE_INDEXER (self), NULL); type check for file, and !cancellable || G_IS_CANCELLABLE (cancellable) If build_flags can be NULL, then set (nullable) in the gtk-doc, otherwise NULL check. @@ +107,3 @@ + * @self: A #IdeCodeIndexer instance. + * @location: Source location of refernece. + * @cancellable: (allow-none): A #GCancellable. allow-none is depreciated, use (nullable) @@ +122,3 @@ + IdeCodeIndexerInterface *iface; + + g_return_if_fail (IDE_IS_CODE_INDEXER (self)); parameter checks location/cancellable too ::: libide/symbols/ide-code-indexer.h @@ +52,3 @@ + GCancellable *cancellable, + GError **error); +void ide_code_indexer_get_key_async (IdeCodeIndexer *self, We should avoid "get" names for non-accessor functions. Maybe something like build/create/make/generate key.
Review of attachment 358274 [details] [review]: Looks good, but there are still some improvements we can make to ensure that this is in good shape to use for future projects. ::: plugins/code-index/ide-persistent-map-builder.c @@ +84,3 @@ + + /* + * Key in hash table will point to actual key string in byte array. This is confusing or inaccurate. It can't be in the byte array (as the byte array could be resized and moved in memory) and it it looks like it's strdup()'d anyway? If it has changed, the comment needs updating. @@ +138,3 @@ + g_variant_dict_init (&dict, NULL); + + keys = g_variant_new_fixed_array ((const GVariantType *) "y", G_VARIANT_TYPE_BYTE @@ +144,3 @@ + + values = g_variant_new_array (NULL, + (GVariant * const *)self->values->pdata, You can possibly get cast warnings here due to alignment requirements, so do: (GVariant * const *)(gpointer)self->values->pdata @@ +148,3 @@ + + g_array_sort_with_data (self->kvpairs, (GCompareDataFunc)compare, self->keys->data); + kvpairs = g_variant_new_fixed_array ((const GVariantType *) "(uu)", G_VARIANT_TYPE ("(uu)") which in addition to casting, does a format type check. @@ +180,3 @@ + g_task_return_boolean (task, TRUE); + else + g_task_return_error (task, error); I want to see us be very explicit in the code-base about ownership transfers, especially for errors. I've fixed a number of bugs that would have been easier to find if they used g_autoptr(GError) error = NULL; and g_steal_pointer() when ownership transferred to the called function. @@ +191,3 @@ +{ + g_autoptr(GTask) task = NULL; + Always to precondition checks. @@ +223,3 @@ + g_task_set_source_tag (task, ide_persistent_map_builder_write_async); + + ide_thread_pool_push_task (IDE_THREAD_POOL_INDEXER, task, ide_persistent_map_builder_write_worker); No need to use the INDEXER thread pool for this operation, which is just an IO write really. Just use g_task_run_in_thread() which uses the default threadpool (primarily focused around doing I/O). @@ +271,3 @@ + +IdePersistentMapBuilder* +ide_persistent_map_builder_new () (void) ::: plugins/code-index/ide-persistent-map.c @@ +49,3 @@ + gsize n_kvpairs; + + gboolean loaded : 1; gboolean loaded : 1 is incorrect as gboolean is a *signed* int (for historical reasons). Therefore, you only have a single bit, which is the signed bit. Use "guint loaded : 1;" to be correct here. @@ +135,3 @@ + return; + } + Let's add an endian check here, by looking up a new "byte-order" field in the dict. Set it to self->byte_order. Then in the lookup routine, we can g_variant_byteswap() if necessary. @@ +212,3 @@ + g_task_set_source_tag (task, ide_persistent_map_load_file_async); + + ide_thread_pool_push_task (IDE_THREAD_POOL_INDEXER, task, ide_persistent_map_load_file_worker); g_task_run_in_thread(), as this is just generic I/O. @@ +227,3 @@ +} + +GVariant * Missing a gtk-doc with (transfer full) for the return type. Additionally, if we track the endianness of the creation (which defaults to host endianness), then we can return something that has g_variant_byteswap() so the consumer doesn't need to know about it. @@ +238,3 @@ + + l = 0; + r = self->n_kvpairs - 1; /* unsigned long to signed long */ While this works with GCC/Clang, I'm not sure that this is not undefined behavior without a manual cast to (glong) before the subtraction. (glong)self->n_kvpairs - 1 @@ +268,3 @@ + g_return_val_if_fail (IDE_IS_PERSISTENT_MAP (self), 0); + + value = g_variant_dict_lookup_value (self->metadata, key, G_VARIANT_TYPE_UINT64); You can avoid creating the variant here with: if (g_variant_dict_lookup (self->metadata, key, G_VARIANT_TYPE_UINT64, &v64)) return v64; @@ +304,3 @@ + +IdePersistentMap* +ide_persistent_map_new () (void) ::: plugins/code-index/ide-persistent-map.h @@ +39,3 @@ + GAsyncResult *result, + GError **error); +GVariant *ide_persistent_map_lookup_value (IdePersistentMap *self, Mind the alignment
Sorry about the multiple copies. I blame bugzilla.
Review of attachment 358275 [details] [review]: ::: plugins/code-index/ide-code-index-builder.c @@ +80,3 @@ + +static gboolean +timeval_compare (GTimeVal a, GTimeVal b) One param per line @@ +82,3 @@ +timeval_compare (GTimeVal a, GTimeVal b) +{ + if ((a.tv_sec > b.tv_sec) || ((a.tv_sec == b.tv_sec) && (a.tv_usec > b.tv_usec))) return ((a.tv_sec > b.tv_sec) || ((a.tv_sec == b.tv_sec) && (a.tv_usec > b.tv_usec))) @@ +110,3 @@ + all_files = g_ptr_array_new_with_free_func (g_object_unref); + + for (int i = 0; i < changes->len ; i++) guint i @@ +116,3 @@ + dir_files = ((IndexingData *)g_ptr_array_index (changes, i))->files; + + for (int j = 0; j < dir_files->len; j++) guint j @@ +137,3 @@ + GFile *file, + guint32 file_id, + IdePersistentMapBuilder *map_builder, mind alignment @@ +167,3 @@ + IdeCodeIndexEntry *entry; + + g_snprintf (num, 100, "%d", file_id); don't duplicate sizes, they can get out of sync. sizeof num @@ +195,3 @@ + NULL, + NULL); + Add a check for 0-based begin_line/offset and set them to 1 if so (so that -1 later on stays valid). They shouldn't be, but we can consider them "unset" in this case I guess. @@ +199,3 @@ + ide_persistent_map_builder_insert (map_builder, + usr, + g_variant_new ("(uuuu)", Alignment @@ +264,3 @@ + + if (!ide_persistent_map_builder_write (map_builder, + keys_file, Alignment @@ +343,3 @@ + g_task_set_task_data (dirs_task, GUINT_TO_POINTER (changes->len), NULL); + + for (int i = 0; i < changes->len; i++) guint i @@ +471,3 @@ + } + + for (int i = 0; i < directories->len; i++) guint i @@ +473,3 @@ + for (int i = 0; i < directories->len; i++) + { + gchar *file_name; const @@ +638,3 @@ + + /* Update build_flags hash table with new flags */ + g_hash_table_iter_init (&iter, self->build_flags); This seems incorrect, you're both iterating self->build_flags and inserting back into self->build_flags. Should one of these just be "build_flags"? ::: plugins/code-index/ide-code-index-builder.h @@ +42,3 @@ + GError **error); +IdeCodeIndexBuilder *ide_code_index_builder_new (IdeContext *context, + IdeCodeIndexIndex *index, Mind the alignment.
Review of attachment 358276 [details] [review]: Cool, just some questions to make things more readable in the future. ::: plugins/code-index/ide-code-index-index.c @@ +25,3 @@ +#include "ide-persistent-map.h" + +#define MAX_LEN 20 Too generic of a define, we should be more specific in the naming so it's obvious what it does when reading the define name out of context. @@ +86,3 @@ + g_clear_pointer (&data->query, g_free); + + for (int i = 0; i < data->fuzzy_matches->len; i++) guint @@ +299,3 @@ +static IdeCodeIndexSearchResult * +ide_code_index_index_new_search_result (IdeCodeIndexIndex *self, + FuzzyMatch *fuzzy_match) FuzzyMatch can probably be const? @@ +341,3 @@ + + return ide_code_index_search_result_new (context, + title + 2, Maybe use a define for this? I'm assuming it's the "type prefix" like "c " or something like that? @@ +576,3 @@ + } + + g_snprintf (num, MAX_LEN, "%d", file_id); sizeof num instead of MAX_LEN ::: plugins/code-index/ide-code-index-index.h @@ +29,3 @@ + +IdeCodeIndexIndex *ide_code_index_index_new (IdeContext *context); +/* This function will load index from directory if it is not modified. This fuction will only Comments like this belong in the .c file with the gtk-doc for the function.
Review of attachment 358278 [details] [review]: ::: plugins/clang/ide-clang-code-indexer.c @@ +123,3 @@ + + /* entries has to dispose TU when done with it */ + entries = ide_clang_code_index_entries_new (&tu, filename); Why is a stack address being passed? If it steals ownership, why not just g_steal_pointer (&tu)? @@ +149,3 @@ + g_autofree gchar *key; + IdeSourceLocation *location; + assertion precondition checks
Review of attachment 358281 [details] [review]: ::: plugins/code-index/ide-code-index-search-provider.c @@ +63,3 @@ + IdeCodeIndexService *service; + IdeCodeIndexIndex *index; + GTask *task; g_autoptr(GTask) task = NULL;
Review of attachment 358280 [details] [review]: Use boxed for the :location property, then looks good. ::: plugins/code-index/ide-code-index-search-result.c @@ +65,3 @@ + + case PROP_LOCATION: + g_value_set_pointer (value, self->location); set_boxed @@ +88,3 @@ + + case PROP_LOCATION: + self->location = ide_source_location_ref (g_value_get_pointer (value)); self->location = g_value_dup_boxed (value); @@ +127,3 @@ + + properties [PROP_LOCATION] = + g_param_spec_pointer ("location", This should be a g_param_spec_boxed() with IDE_TYPE_SOURCE_LOCATION
Review of attachment 358286 [details] [review]: ::: libide/buildsystem/ide-build-system.c @@ +116,3 @@ g_steal_pointer (&flags)); + if (++data->index < data->files->len) I don't like increment operators and comparison operators mixed. Keep these separate.
Review of attachment 358287 [details] [review]: ::: libide/ide-context.c @@ +2078,3 @@ NULL); + + g_clear_object (&self->device_manager); This looks brittle to me. We should wait to clear the manager objects as a final async callback step perhaps?
Review of attachment 358288 [details] [review]: Cool
Review of attachment 358285 [details] [review]: Cool
Review of attachment 358284 [details] [review]: Cool
Review of attachment 358282 [details] [review]: Cool
Review of attachment 358279 [details] [review]: I'm concerned that GListModel isn't helping here. Can we stop using GListModel? ::: plugins/clang/ide-clang-code-index-entries.c @@ +42,3 @@ + PROP_0, + PROP_MAIN_FILE, + PROP_TU, This is an unhelpful property name. Maybe "unit" if you think "translation-unit" is too long. @@ +56,3 @@ +static void +cx_cursor_free (CXCursor *cursor, + gpointer user_data) drop user_data from parameters @@ +66,3 @@ + */ +static enum CXChildVisitResult +visitor (CXCursor cursor, CXCursor parent, CXClientData client_data) One parameter per line @@ +68,3 @@ +visitor (CXCursor cursor, CXCursor parent, CXClientData client_data) +{ + IdeClangCodeIndexEntries *self = (IdeClangCodeIndexEntries *)client_data; No cast is necessary -> typedef void *CXClientData; @@ +217,3 @@ + break; + case CXCursor_VarDecl: + kind = IDE_SYMBOL_VARIABLE; break is missing. Remove this kind = and just fallthrough to the following case. @@ +230,3 @@ + kind = IDE_SYMBOL_NAMESPACE; + break; + case CXCursor_FunctionTemplate: Share case blocks for items that use the same kind. case CXCursor_FunctionTemplate: case CXCursor_ClassTemplate: kind = IDE_SYMBOL_TEMPLATE; break; @@ +252,3 @@ + /* Add prefix to name so that filters can be applied */ + if (kind == IDE_SYMBOL_FUNCTION) + prefix = "f\x1F"; We should probably add a new define in ide-macros.h for special string separators. @@ +318,3 @@ + + while (data == NULL && !finish) + data = ide_clang_code_index_entries_real_get_item (model, position, &finish); This seems like a strange way to do this. It seems like it's shoe-horning things into using GListModel. If GListModel isn't helping, we shouldn't implement it. @@ +451,3 @@ + +IdeClangCodeIndexEntries* +ide_clang_code_index_entries_new (CXTranslationUnit *tu, Isn't CXTranslationUnit already a pointer? Why add *? ::: plugins/clang/ide-clang-code-index-entries.h @@ +29,3 @@ +G_DECLARE_FINAL_TYPE (IdeClangCodeIndexEntries, ide_clang_code_index_entries, IDE, CLANG_CODE_INDEX_ENTRIES, GObject) + +IdeClangCodeIndexEntries *ide_clang_code_index_entries_new (CXTranslationUnit *tu, Let's drop the * on tu, it's already a pointer. (from Index.h) typedef struct CXTranslationUnitImpl *CXTranslationUnit; @@ +30,3 @@ + +IdeClangCodeIndexEntries *ide_clang_code_index_entries_new (CXTranslationUnit *tu, + gchar *source_filename); const gchar *source_filename ::: plugins/clang/ide-clang-code-index-entry.c @@ +63,3 @@ + g_return_val_if_fail (IDE_IS_CLANG_CODE_INDEX_ENTRY (self), NULL); + + return self->usr; return g_strdup (self->usr) (as we want interfaces to return string copies so they work with PyGObject correctly) ::: plugins/clang/ide-clang-code-index-entry.h @@ +38,3 @@ + guint end_line_offset); + +IdeClangCodeIndexEntry *ide_clang_code_index_entry_new (); (void)
Review of attachment 358283 [details] [review]: Couple correctness things to fix, then this looks good. Nice work. ::: libide/plugins/ide-extension-set-adapter.c @@ +210,3 @@ } } + g_signal_emit (self, signals [EXTENSIONS_LOADED], 0, self); miss-alignment and trailing "self" should not be necessary. @@ +455,3 @@ + G_TYPE_NONE, + 1, + G_SIGNAL_RUN_LAST, This isn't right, self is already passed as the first argument, so this should be: G_TYPE_NONE, 0); ::: libide/sourceview/ide-source-view.c @@ +6009,3 @@ { + IdeSymbolResolver *symbol_resolver = (IdeSymbolResolver *)object; + g_autoptr(IdeSourceView) self; self should not be an autoptr, g_task_get_source_object() does not return a new reference. ::: tests/meson.build @@ +17,3 @@ 'test-ide-context.c', c_args: ide_test_cflags, + dependencies: [ Create ide_test_deps and then use that like we do for cflags.
Created attachment 358473 [details] [review] code-index: Adding IdeCodeIndexService to code-index fixed
Created attachment 358474 [details] [review] symbols: Adding IdeCodeIndexer,IdeCodeIndexEntry interfaces fixed
Created attachment 358475 [details] [review] Adding persistent map fixed
Created attachment 358476 [details] [review] code-index: Adding IdeCodeIndexBuilder fixed
Created attachment 358477 [details] [review] code-index: Adding IdeCodeIndexIndex fixed
Created attachment 358478 [details] [review] clang: Implement IdeCodeIndexer fixed
Created attachment 358479 [details] [review] code-index: Implementing IdeSearchResult fixed
Created attachment 358480 [details] [review] code-index: Implmenting search provider fixed
Created attachment 358481 [details] [review] code-index: Implementing symbol resolver fixed
Created attachment 358482 [details] [review] buffer: Use multiple symbol resolvers instead of one. fixed
Created attachment 358483 [details] [review] buildsystem: Handle empty file array and task cancellation fixed
Created attachment 358484 [details] [review] libide: Unload services before destroying RuntimeManager fixed
Created attachment 358485 [details] [review] symbols: Adding IdeCodeIndexEntries interface New interface added.
Created attachment 358486 [details] [review] clang: Implement IdeCodeIndexEntries and IdeCodeIndexEntry
Created attachment 358500 [details] [review] code-index: fixes in IdeCodeIndexBuilder and IdePersistentMap few bugs are fixed. When IdeCodeIndexer is null in IdeCodeIndexBuilder dont proceed to index file. When target value is found in IdePersistentMap then stop searching.
Review of attachment 358473 [details] [review]: Fix the crasher and then this should be good to go ::: plugins/code-index/ide-code-index-service.c @@ +123,3 @@ + bdata->directory, + bdata->recursive, + self->cancellable, This is going to crash. You need to make the GCancellable a local because self will be made NULL before this dereference occurs.
Review of attachment 358474 [details] [review]: LGTM
Review of attachment 358475 [details] [review]: LGTM
Review of attachment 358476 [details] [review]: Couple nits, otherwise LGTM ::: plugins/code-index/ide-code-index-builder.c @@ +165,3 @@ + IdeCodeIndexEntry *entry; + + g_snprintf (num, 100, "%u", file_id); 100 -> sizeof num @@ +207,3 @@ + name, + g_variant_new ("(uuuuu)", + file_id, Indent the g_variant_new() parameters so it is clear they are part of it ::: plugins/code-index/ide-code-index-builder.h @@ +42,3 @@ + GError **error); +IdeCodeIndexBuilder *ide_code_index_builder_new (IdeContext *context, + IdeCodeIndexIndex *index, Alignment
Review of attachment 358477 [details] [review]: LGTM
Review of attachment 358478 [details] [review]: LGTM
Review of attachment 358479 [details] [review]: LGTM
Review of attachment 358480 [details] [review]: LGTM
Review of attachment 358481 [details] [review]: LGTM
Review of attachment 358482 [details] [review]: LGTM ::: tests/meson.build @@ +15,3 @@ +ide_test_deps = [ + libpeas_dep, add libide_dep here and then remove it from the entries below
Review of attachment 358483 [details] [review]: LGTM
Review of attachment 358484 [details] [review]: Hrmm, I was thinking of adding a new unload async cb for unref'ing these, but this seems fine too.
Review of attachment 358485 [details] [review]: Adjust things to allow a full reference return (possibly in consuming patches too) and then this is good. ::: libide/symbols/ide-code-index-entries.c @@ +41,3 @@ + * This will fetch next entry in index. + * + * Returns: (transfer none) : An #IdeCodeIndexEntry. We need a (transfer full) here so that we can do lazy expansion of items by creating each Entry object on demand.
Review of attachment 358486 [details] [review]: Couple quick things, and adjust for the (transfer full) ::: plugins/clang/ide-clang-code-index-entries.c @@ +308,3 @@ + entry = ide_clang_code_index_entries_real_get_next_entry (self, &finish); + + return (IdeCodeIndexEntry *)entry; Return a g_object_ref(self->entry) here so that we allow ourselves some flexibility in the future to create new objects on demand. @@ +339,3 @@ + G_OBJECT_CLASS (ide_clang_code_index_entries_parent_class)->constructed (object); + + root_cursor = g_slice_new (CXCursor); You can also do something like: CXCursor cursor; cursor = clang_getTranslationUnitCursor (self->tu); g_queue_push_head (&self->cursors, g_slice_dup (CXCursor, &cursor)); @@ +399,3 @@ + + properties [PROP_MAIN_FILE] = + g_param_spec_pointer ("main_file", - instead of _ for property names @@ +433,3 @@ + return g_object_new (IDE_TYPE_CLANG_CODE_INDEX_ENTRIES, + "unit", unit, + "main_file", main_file, I know that - and _ both work for property names, but use - to be consistent.
Review of attachment 358500 [details] [review]: LGTM
Created attachment 358695 [details] [review] symbols: Adding IdeCodeIndexEntries interface complete transfer of IdeCodeIndexEntry.
Created attachment 358696 [details] [review] symbols: Adding IdeCodeIndexer & IdeCodeIndexEntry IdeCodeIndexEntry is now a class instead of interface. Implementation of IdeCodeIndexEntries will return an instance of IdeCodeIndexEntry by setting properties in it.
Created attachment 358697 [details] [review] clang: Implement IdeCodeIndexEntries IdeClangCodeIndexEntries now return an instance of IdeCodeIndexEntry (transfer full).
Created attachment 358698 [details] [review] code-index: Adding IdeCodeIndexBuilder fixed.
Created attachment 358699 [details] [review] code-index: Adding IdeCodeIndexService to code-index error using g_steal_pointer is fixed.
Created attachment 358700 [details] [review] clang: Adding code indexer IdeClangCodeIndexEntry class files removed.
Created attachment 358705 [details] [review] buffer: Use multiple symbol resolvers instead of one. libide_dep added to ide_test_deps in tests/meson.build
Comment on attachment 358500 [details] [review] code-index: fixes in IdeCodeIndexBuilder and IdePersistentMap Changes are included in this https://bugzilla.gnome.org/show_bug.cgi?id=786700#c74 patch.
I'm getting an infinite loop somewhere due to this. I think it starts from ide_code_index_symbol_resolver_lookup_cb()
+ Trace 237918
Created attachment 358962 [details] [review] code-index: fix infinte loop in binary search
Excellent work, I'm really happy to see this merged. I think it's in good enough shape that if/when we find issues, we can just keep plugging away at things on the master branch. Attachment 358272 [details] pushed as 67f3085 - symbols: Added symbols, flags and icons Attachment 358284 [details] pushed as 35f35a6 - code-index: Adding plugin files Attachment 358475 [details] pushed as 103df38 - Adding persistent map Attachment 358477 [details] pushed as 73c8e7e - code-index: Adding IdeCodeIndexIndex Attachment 358478 [details] pushed as bd27c6e - clang: Implement IdeCodeIndexer Attachment 358479 [details] pushed as 8a38038 - code-index: Implementing IdeSearchResult Attachment 358480 [details] pushed as ac3eb3e - code-index: Implmenting search provider Attachment 358481 [details] pushed as 8056ad5 - code-index: Implementing symbol resolver Attachment 358483 [details] pushed as 0cc6d80 - buildsystem: Handle empty file array and task cancellation Attachment 358695 [details] pushed as 80118f9 - symbols: Adding IdeCodeIndexEntries interface Attachment 358696 [details] pushed as a859535 - symbols: Adding IdeCodeIndexer & IdeCodeIndexEntry Attachment 358697 [details] pushed as 816d550 - clang: Implement IdeCodeIndexEntries Attachment 358698 [details] pushed as 4e77909 - code-index: Adding IdeCodeIndexBuilder Attachment 358699 [details] pushed as aec820a - code-index: Adding IdeCodeIndexService to code-index Attachment 358700 [details] pushed as 0ac0135 - clang: Adding code indexer Attachment 358705 [details] pushed as 6f17acd - buffer: Use multiple symbol resolvers instead of one. Attachment 358962 [details] pushed as fb924f1 - code-index: fix infinte loop in binary search
Created attachment 358975 [details] [review] code-index: Avoid infinite build loop Added a variable which keeps track of number of trials are made for bulding index. If current trial number exceeds maximum then next trial for building wont be made.
Created attachment 358981 [details] [review] code-index: Avoid segfault when indexing is stopped
Review of attachment 358975 [details] [review]: LGTM
Review of attachment 358981 [details] [review]: LGTM
Comment on attachment 358975 [details] [review] code-index: Avoid infinite build loop Attachment 358975 [details] pushed as 240bca3 - code-index: Avoid infinite build loop