D9016: remove global statics

[Francis Herne]

flherne requested changes to this revision.
flherne added a comment.
This revision now requires changes to proceed.


  This is definitely nicer; I remember being a bit puzzled by Helper being a class, and the inscrutable static-var problems are a nuisance.
  
  There are some pre-existing thread-safety issues that should probably be fixed here.

INLINE COMMENTS

> helpers.cpp:80
> +
> +QStringList& Helper::dataDirs() {
> +    static QStringList value;

Is this necessary? The variable is only used in `getDataDirs()`. Some of the others are similar.

> helpers.cpp:105
> +
> +QMutex& Helper::cacheMutex() {
> +    static QMutex value;

Can these be next to the variable-containing-function they protect?

Does it make sense for this to protect both `cachedSearchPaths()` and `cachedCustomIncludes()`? They don't seem particularly related.

> helpers.cpp:325
> +    if ( Helper::dataDirs().isEmpty() ) {
> +        Helper::dataDirs() = QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, "kdevpythonsupport/documentation_files", QStandardPaths::LocateDirectory);
>      }

This doesn't look thread-safe (in either the old or modified version); nothing locks the value while it's being written to.

> helpers.cpp:334
>          auto path = QStandardPaths::locate(QStandardPaths::GenericDataLocation, "kdevpythonsupport/documentation_files/builtindocumentation.py");
> -        Helper::documentationFile = IndexedString(path);
> +        Helper::documentationFile() = IndexedString(path);
>      }

ditto

> helpers.cpp:348
>          ReferencedTopDUContext ctx = ReferencedTopDUContext(DUChain::self()->chainForDocument(file));
> -        Helper::documentationFileContext = DUChainPointer<TopDUContext>(ctx.data());
> +        Helper::documentationFileContext() = DUChainPointer<TopDUContext>(ctx.data());
>          return ctx;

ditto

> helpers.cpp:391
> +    if ( Helper::correctionFileDirs().isEmpty() ) {
> +        Helper::correctionFileDirs() = QStandardPaths::locateAll(QStandardPaths::GenericDataLocation,
>                                                                 "kdevpythonsupport/correction_files/",

ditto

> helpers.cpp:416
> +    if ( Helper::localCorrectionFileDir().isNull() ) {
> +        Helper::localCorrectionFileDir() = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) + QLatin1Char('/') + "kdevpythonsupport/correction_files/";
>      }

ditto...

REPOSITORY
  R53 KDevelop: Python Support

REVISION DETAIL
  https://phabricator.kde.org/D9016

To: brauch, flherne
Cc: kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171127/0b0ffd23/attachment-0002.html>