Bug 478198 - Umbrello import wizard does not recognize the 'override' C++ keyword
Summary: Umbrello import wizard does not recognize the 'override' C++ keyword
Status: RESOLVED FIXED
Alias: None
Product: umbrello
Classification: Applications
Component: importer (show other bugs)
Version: Git
Platform: Arch Linux Linux
: NOR major
Target Milestone: ---
Assignee: Umbrello Development Group
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-07 09:30 UTC by Nicola Mori
Modified: 2023-12-23 17:18 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.38.80 (KDE releases 24.01.80)
Sentry Crash Report:


Attachments
Screenshot showing imported classifiers I and C (13.18 KB, image/png)
2023-12-07 14:51 UTC, Oliver Kellogg
Details
Import of full project (see comment 2) with override keywords. (68.38 KB, image/png)
2023-12-09 17:34 UTC, Nicola Mori
Details
Import of full project (see comment 2) without override keywords. (178.49 KB, image/png)
2023-12-09 17:37 UTC, Nicola Mori
Details
test.h from test.tar at https://basket.fi.infn.it:443/f/ee0c58a81b/?dl=1 (10.10 KB, text/x-chdr)
2023-12-10 20:01 UTC, Oliver Kellogg
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nicola Mori 2023-12-07 09:30:03 UTC
SUMMARY
When importing C++ code containing the 'override' keyword Umbrello either does not import the class, or imports it with missing methods, or the overall import is screwed. A simple reproducer is available below; correct behavior can be restored by removing the `override` keyword.

This seems to be a regression since a similar bug has been fixed in https://bugs.kde.org/show_bug.cgi?id=386258.

Using Umbrello 2.38.3 (Applications 23.08.3); this version is not listed in the `Version` list of the bug tracker so I chose `Git`.

STEPS TO REPRODUCE
1. Create a simple header file like this:
```
class I {
public:
  virtual void Method() = 0;
};

class C: public I {
public:
  void Method() override  {}
};
```
2. Import it with the Code Importing Wizard 
3. Verify that class `I` is parsed in the import logger tab

OBSERVED RESULT
Class `I` is not available in the Tree View

EXPECTED RESULT
Class `I` should be available

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: ArchLinux
KDE Plasma Version: 5.27.10
KDE Frameworks Version: 5.112.0
Qt Version: 5.15.11

ADDITIONAL INFORMATION
Comment 1 Oliver Kellogg 2023-12-07 14:51:50 UTC
Created attachment 163981 [details]
Screenshot showing imported classifiers I and C

Cannot reproduce, see attached screenshot.
Comment 2 Nicola Mori 2023-12-09 17:34:56 UTC
Created attachment 164045 [details]
Import of full project (see comment 2) with override keywords.
Comment 3 Nicola Mori 2023-12-09 17:37:48 UTC
Created attachment 164046 [details]
Import of full project (see comment 2) without override keywords.
Comment 4 Nicola Mori 2023-12-09 17:38:31 UTC
(In reply to Oliver Kellogg from comment #1)
> Created attachment 163981 [details]
> Screenshot showing imported classifiers I and C
> 
> Cannot reproduce, see attached screenshot.
Nor can I... It's weird, I accurately tested the reproducer before opening the bug report and I'm 100% sure it behaved as I wrote in my first post.

Anyway, I can still reproduce garbage behavior with my original header file. You can download it here: 

https://basket.fi.infn.it:443/f/ee0c58a81b/?dl=1)

together with a cpp file for compiling everything and check that it's correct code. Sorry for the poor overall state of the code but it's just a playground for testing ideas, and I was trying to extract the UML with Umbrello after verifying that the overall design works for me.
When importing the test.h header the AlgoSequenceDecorator class is imported without any method, and the AlgoSequence and MTScheduler classes are not imported at all. Removing all the override keywords (sed -i 's/override//g' test.h) the AlgoSequenceDecorator is imported with all its methods, and also AlgoSequence is now imported; however MTScheduler is stil missing, so probably there's more than just override. I uploaded two screenshots showing this behavior.

The code compiles fine both with and without the override keywords, so it's valid and thus I think this is a problem with Umbrello.
Comment 5 Oliver Kellogg 2023-12-10 08:36:35 UTC
Your full header file is helpful indeed, I can now reproduce problems.

Here is the current status of import:

class COUTC : public std::ostream {                                OK
class Configurable /*: public IConfigurable*/ {                    NOT IMPORTED
class IAlgorithm : public Configurable {                           OK
class Algorithm : public IAlgorithm {                              OK
class ConcreteAlgorithm : public Algorithm {                       OK
class AlgoSequenceDecorator;                                       OK
class IAlgoExecutor {                                              OK
class PlainAlgoExecutor : public IAlgoExecutor {                   OK
class AlgoExecutorDecorator : public IAlgoExecutor {               OK
class IAlgoScheduler {                                             OK
class SequentialScheduler : public IAlgoScheduler {                NOT IMPORTED
class MTScheduler : public IAlgoScheduler {                        NOT IMPORTED
class IAlgoSequence : public IAlgorithm {                          OK
class AlgoSequence : public IAlgoSequence {                        NOT IMPORTED
class AlgoSequenceDecorator : public IAlgoSequence {               NOT IMPORTED
class SchedulerDecorator : public IAlgoScheduler {                 OK
class GarbageCollectorDecorator : public AlgoSequenceDecorator {   NOT IMPORTED
  class GarbageCollectorScheduler : public SchedulerDecorator {    NOT IMPORTED
  class GarbageCollectorExecutor : public AlgoExecutorDecorator {  OK

Messages in Log window: 
      
test.h:58:109: error: ';' expected found '}'
test.h:58:110: error: Syntax error before ')'
test.h:59:14: error: ';' expected found 'GetParameter'
test.h:68:0: error: Syntax error before 'private'
test.h:70:0: error: Syntax error before '}'

Those lines are in class Configurable:
58:  template <class T> void SetParameter(std::string name, T val) { _params.push_back({name, typeid(val).name()}); }
59:  std::string GetParameter(std::string name) /* override*/ {

The actual problem is
    _params.push_back({name, typeid(val).name()}); 
where the parser is not handling the ad hoc pair construct
    {name, typeid(val).name()}
Comment 6 Oliver Kellogg 2023-12-10 20:01:58 UTC
Created attachment 164078 [details]
test.h from test.tar at https://basket.fi.infn.it:443/f/ee0c58a81b/?dl=1
Comment 7 Oliver Kellogg 2023-12-10 20:46:25 UTC
Git commit ee83f8db682b8050038034cdd41d15c8f8cbed3c by Oliver Kellogg.
Committed on 10/12/2023 at 21:46.
Pushed by okellogg into branch 'master'.

lib/cppparser/parser.cpp : Address problems uncovered by https://bugs.kde.org/attachment.cgi?id=164078

- In function skip(int l, int r) remove special handling for case
     l != '{' && (tk == '{' || tk == '}' || tk == ';')
- In function skipExpression handle case '{' by skip('{', '}').
- In function parsePtrOperator handle case Token_and (reference
  operator).
- In function parseMemInitializer handle { } in addition to ( ) as
  delimiters for initialization expression.
- In function parseFunctionBody use skip('{', '}') in lieu of detailed
  function body statement representations.
- In function parsePrimaryExpression add case Token_default for
  supporting usages like ~IAlgoScheduler() = default.
FIXED-IN: 2.38.80 (KDE releases 24.01.80)

M  +32   -9    lib/cppparser/parser.cpp

https://invent.kde.org/sdk/umbrello/-/commit/ee83f8db682b8050038034cdd41d15c8f8cbed3c
Comment 8 Nicola Mori 2023-12-11 07:58:17 UTC
Thank you very much Oliver.
Comment 9 Oliver Kellogg 2023-12-23 17:18:45 UTC
(In reply to Nicola Mori from comment #8)
> Thank you very much Oliver.

Sure.
Feel free to reopen or open new bug in case of further problems.