Bug 397666

Summary:	C++ importer does not recognize 'final' keyword
Product:	[Applications] umbrello	Reporter:	Thomas Jansen <mithi>
Component:	general	Assignee:	Umbrello Development Group <umbrello-devel>
Status:	RESOLVED FIXED
Severity:	normal	CC:	okellogg
Priority:	NOR
Version First Reported In:	Git
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://invent.kde.org/sdk/umbrello/commit/6c8c82400ae92fbb307a298e4231b607dfa8b369	Version Fixed/Implemented In:	2.33.80 (KDE releases 22.03.80)
Sentry Crash Report:
Attachments:	Initial patch

Description Thomas Jansen 2018-08-20 16:15:10 UTC

Created attachment 114519 [details]
Initial patch

When trying to import existing C++ code that uses the 'final' keyword introduced in C++11 (https://en.cppreference.com/w/cpp/language/final), umbrello fails to parse it correctly. Apparently the lexer doesn't know about the final keyword.

Final can occur in two places:
1. class definition (e.g. "class foo final {...};") 
2. member function declaration "virtual void foo() final;"

Case 1: The following code is imported incorrectly:
class foo final {...};
class bar final {...};

I would expect the C++ import parser to give me two classes named 'foo' and 'bar', instead, it gives me one class named 'final'.

Case 2: The member function declaration is also not correctly parsed, leading to an empty list of member functions for a class that has at least one member function decorated with 'final'.

The attached patch introduces a token for the final keyword to the lexer and uses that token in the parser for the places it may appear. This patch is only the starting point for fixing the bug. In particular the part in Parser::parseDeclarator is not really elegant and needs to be corrected. The problem is that 'final' and 'override' may both be used on a declarator and the order is arbitrary. However, each may only be specified once. This check for duplication is not yet in the patch. Feel expand the code as you see fit. Also this information is not forwarded to other parts of umbrello, e.g. there is no GUI frontend for the parsed final keywords. For my purposes it is enough to parse existing C++ code correctly and display UML class diagrams.

Proposed test case for part 1: The parser should create a class called 'foo' rather than 'final' on the following code:

class foo final {
public:
  foo() {};
  ~foo() {};
};

Proposed test case for part 2: The parser should create a declarator list with the four functions 'a', 'b', 'c', and 'd' for the following code:
class foo {
public:
  virtual bool a() const;
  virtual bool b() const override;
  virtual bool c() const final override;
  virtual bool d() const override final;
};

Please note that function d is not yet properly parsed by my patch.

Comment 1 Oliver Kellogg 2022-02-15 21:41:04 UTC

Git commit 6c8c82400ae92fbb307a298e4231b607dfa8b369 by Oliver Kellogg.
Committed on 15/02/2022 at 21:38.
Pushed by okellogg into branch 'master'.

Fix "C++ importer does not recognize 'final' keyword"

Apply final-keyword.patch by Thomas Jansen with modifications:

lib/cppparser/lexer.h
- In enum Type add Token_final.

lib/cppparser/keywords.h
- Add INSERT("final", Token_final).

lib/cppparser/ast.{h,cpp}
- In class ClassSpecifierAST,
  - add getter final_() returning m_final.get();
  - add function setFinal(AST::Node& final_);
  - add private member m_final of type AST::Node.
- In class DeclaratorAST,
  - rename getter override() to override_() for avoiding incorrect
    colorization in syntax highlighting editors;
  - add getter final_() returning m_final.get();
  - add function setFinal(AST::Node& final_);
  - add private member m_final of type AST::Node.

lib/cppparser/ast_utils.cpp
- In function declaratorToString,
  - adjust call of declarator->override() to function renaming;
  - add handling of (declarator->final_() != 0).

lib/cppparser/parser.cpp
- Remove #include "optionstate.h" and testing of
  Settings::optionState().codeImportState.supportCPP11.
  Reason: In the year 2022 there is little use for switching off C++11,
  in particular in our usecase within a UML tool not compiler frontend.
- In functions parseDeclarator and parseAbstractDeclarator, after
  skipping ')' of parameter declaration clause add loop for handling
  Token_const, Token_override, Token_final.  These are handled in a
  loop because they may appear in arbitrary order.
- In function parseClassSpecifier,
  - after parsing name set AST::Node final_ if Token_final is present;
  - call ast->setFinal(final_).

umbrello/umlmodel/operation.{h,cpp}
- Add public functions setFinal(bool) and bool getFinal().
- Rename private member m_Override to m_bOverride, m_virtual to
  m_bVirtual, m_inline to m_bInline for symmetry with m_bConst.
- Add private member m_bFinal of type bool.

umbrello/umlmodel/operation.cpp
- In UMLOperation constructor initialize m_bFinal.
- In function saveToXMI :
  - Only write XML attibutes "isQuery", "isOverride", "isVirtual",
    "isInline" if the corresponding data member is true.
    This saves space and avoids the non standard attributes when
    possible.
  - Write XML attribute "isFinal" if m_bFinal is true.
- In function load1 add retrieving of m_bFinal from XML attribute
  "isFinal".

umbrello/codeimport/kdevcppparser/cpptree2uml.cpp
- In functions parseFunctionDefinition and parseFunctionDeclaration
  call m->setFinal(true) if d->final_() returns true.

umbrello/version.h
- Increase XMI1_FILE_VERSION to "1.7.5" and increase XMI2_FILE_VERSION
  to "2.0.2" to reflect the added UML Operation attribute "isFinal".

These changes were tested by importing the file
test/import/cxx/cxx11-explicit-overrides-and-final.h
FIXED-IN:2.33.80 (KDE releases 22.03.80)

M  +15   -2    lib/cppparser/ast.cpp
M  +18   -2    lib/cppparser/ast.h
M  +4    -1    lib/cppparser/ast_utils.cpp
M  +1    -0    lib/cppparser/keywords.h
M  +1    -0    lib/cppparser/lexer.h
M  +33   -33   lib/cppparser/parser.cpp
M  +6    -2    umbrello/codeimport/kdevcppparser/cpptree2uml.cpp
M  +45   -19   umbrello/umlmodel/operation.cpp
M  +7    -4    umbrello/umlmodel/operation.h
M  +2    -2    umbrello/version.h

https://invent.kde.org/sdk/umbrello/commit/6c8c82400ae92fbb307a298e4231b607dfa8b369

Comment 2 Oliver Kellogg 2022-02-19 07:49:37 UTC

Git commit 4f6cb3dae55c36bb446e9941dd6cb9bb6320ccac by Oliver Kellogg.
Committed on 19/02/2022 at 07:49.
Pushed by okellogg into branch 'master'.

test/import/cxx/cxx11-explicit-overrides-and-final.h followup to commit 6c8c824 :

- In struct Base2 integrate proposed test case for part 2 from the
  description by Thomas Jansen in which permutations of the qualifiers
  "final", "const", and "override" are tested.

M  +2    -0    test/import/cxx/cxx11-explicit-overrides-and-final.h

https://invent.kde.org/sdk/umbrello/commit/4f6cb3dae55c36bb446e9941dd6cb9bb6320ccac