Fix shadowing of ClassMemberDeclaration::isAbstract by ClassFunctionDeclaration
ClosedPublic

Authored by kossebau on Aug 2 2017, 10:40 PM.

Details

Summary

816bf9de8b91314965de27fb5e855eb0d7486871 added an isAbstract property
to ClassMemberDeclaration, while its subclass ClassFunctionDeclaration
already has had such a property, with same getter, but different setter.
By commit message this was done to support Java language features.

As a result e.g. clang builder sets the property stored with
ClassMemberDeclaration, while other code seeing the subclass
then fetches the not set property stored with ClassFunctionDeclaration.

This patch fixes this by removing the abstract property again from
ClassMemberDeclaration. Being abstract/final is something not possible
in general for any kind of members, but only subtypes like e.g. functions.
(Besides, the final attribute for Java variables has the same keyword,
but is a different concept, so should get stored with a different property)

Test Plan

Code paths using isAbstract() still work or rather work properly now,
like the overrides page in the file template generator now properly
listing abstract methods as abstract.

Diff Detail

Repository
R33 KDevPlatform
Branch
removeShadowingDuplicatedAbstractProperty
Lint
No Linters Available
Unit
No Unit Test Coverage
kossebau created this revision.Aug 2 2017, 10:40 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptAug 2 2017, 10:40 PM

Target: 5.1, for master the respective methods would be simply dropped from ClassFunctionDeclaration.

kfunk added a subscriber: kfunk.Aug 3 2017, 8:49 AM

Hmm, why don't target master branch directly? I wouldn't even touch 5.1 for this.

IMO there's no need to mark them deprecated. We can/should fix the potential breakage in the language plugins we have access to ourselves immediately.

In D7084#131686, @kfunk wrote:

Hmm, why don't target master branch directly? I wouldn't even touch 5.1 for this.

IMO there's no need to mark them deprecated. We can/should fix the potential breakage in the language plugins we have access to ourselves immediately.

Okay. No real oversight where this has an effect, besides the filetemplate plugin generator where currently abstract methods are not discovered and getting the special handling prepared.
Though not really a grave error, and actually uncovers another issue as that selected methods are passed on and then get rendered in the generated files as is, so abstract methods would still be shown as abstract. And given no-one else caught this yet, nothing else seems affected.

So will rework patch as fix on master, i.e. remove the property from ClassMemberDeclaration (together with the incomplete bits of final). Should be up later today.

kossebau updated this revision to Diff 17629.Aug 3 2017, 2:49 PM

update patch for master

kossebau edited the summary of this revision. (Show Details)Aug 3 2017, 2:49 PM

Wouldl remove the unused bit for Final property in ClassMemberDeclaration in a follow-up commit.

kfunk accepted this revision.Aug 3 2017, 2:55 PM

And +1 on removing isFinal, too.

This revision is now accepted and ready to land.Aug 3 2017, 2:55 PM
This revision was automatically updated to reflect the committed changes.