D16773: Don't add 'override' specifier for non-modern project settings

[Amish Naidu]

amhndu created this revision.
Herald added a project: KDevelop.
Herald added a subscriber: kdevelop-devel.
amhndu requested review of this revision.

REVISION SUMMARY
Override code completion will only add the override specifier if the parser
settings don't have either -std=c++03 or 98.

Supersedes: D4039 <https://phabricator.kde.org/D4039>

BUG: 372280

TEST PLAN
Run `test_codecompletion testOverrideExecute`

REPOSITORY
R32 KDevelop

BRANCH
config-override

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

AFFECTED FILES
plugins/clang/codecompletion/context.cpp
plugins/clang/tests/test_codecompletion.cpp
plugins/clang/tests/test_codecompletion.h

To: amhndu
Cc: kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Amish Naidu]

amhndu updated this revision to Diff 45160.
amhndu added a comment.


- Change test case and convert octal literal to decimal

REPOSITORY
R32 KDevelop

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D16773?vs=45154&id=45160

BRANCH
config-override

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

AFFECTED FILES
plugins/clang/codecompletion/context.cpp
plugins/clang/tests/test_codecompletion.cpp
plugins/clang/tests/test_codecompletion.h

To: amhndu
Cc: kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Amish Naidu]

amhndu added a comment.


ping ?

REPOSITORY
R32 KDevelop

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

To: amhndu
Cc: kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Kevin Funk]

kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


LGTM in general. If you prefer your version (instead of the one proposed by me) feel free to push directly.

INLINE COMMENTS

context.cpp:186
+ project->filesForPath(IndexedString(view->document()->url().path())).first());
+ const auto match = QRegularExpression(QStringLiteral(R"(-std=c\+\+(\d+))")).match(arguments);
+

The correct way to capture this would probably be `-std=c\+\+(\w+)`, to also capture potential things like "-std=c++1x" (though you don't check that here). Still feels cleaner.

context.cpp:191
+ const int standard = match.captured(1).toInt();
+ appendSpecifer = (standard != 98 && standard != 3);
+ }

And consecutively here: `(standard != "98" && standard != "03")`

Was a little confused by the `3` here at first. I think the string version makes it clearer.

test_codecompletion.cpp:1479
<< "class Foo { int** bar(int x); };\nint ** Foo::bar(int x)\n{\n}\n";
+
}

Unrelated.

REPOSITORY
R32 KDevelop

BRANCH
config-override

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

To: amhndu, kfunk
Cc: kfunk, kdevelop-devel, glebaccon, hase, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd