News:

As usual while waiting for the next release - don't forget to check the nightly builds in the forum.

Main Menu

Patch 3407: correctly parse c++98 enums

Started by oBFusCATed, January 20, 2013, 11:02:31 PM

Previous topic - Next topic

oBFusCATed

Can someone with better knowledge review this patch? Is it possible to add tests for it?
This is a major problem of the current parser and I'd be happy if it is fixed.

I'll apply the patch on my work version of C::B and will give it some testing.

Probably C++11's strong enums (fake) parsing will be broken by this patch, but for me supporting the old enums is preferable.

https://developer.berlios.de/patch/?func=detailpatch&patch_id=3407&group_id=5358
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

killerbot


oBFusCATed

Quote from: killerbot on January 21, 2013, 07:33:00 AM
please keep the enum class also working.
Does they really work or it is just illusion, because of the bug? :)
Apply the patch and test. Then provide feedback...

p.s. I knew you'll would post this :)
p.p.s. c++11 enums are still horrible, the improvement is close to none. (don't flame on this, I don't care)
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

MortenMacFly

Quote from: killerbot on January 21, 2013, 07:33:00 AM
please keep the enum class also working.
Dunno what that means?! What class?

Quote from: oBFusCATed on January 21, 2013, 09:28:52 AM
Does they really work or it is just illusion, because of the bug? :)
Applied in my local copy... test cases can be added here, if you like:
https://svn.berlios.de/svnroot/repos/codeblocks/trunk/src/plugins/codecompletion/testing/structs_typedefs.cpp
...renaming it to something like structs_typedefs_enums.cpp ... harhar...

Will report back...
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]

killerbot

I will test the tomorrow, to see if it still works for enum class.
Will report back ...

thomas

Quote from: MortenMacFly on January 21, 2013, 04:49:25 PM
Quote from: killerbot on January 21, 2013, 07:33:00 AM
please keep the enum class also working.
Dunno what that means?! What class?
C++11 strongly typed enumerations: enum class.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

killerbot

I have tested : this patch is NOT acceptable. It breaks C++11 enum classes.

I think it is better to hand this problem over to our CC gurus, and have them, fix it properly.


#include <iostream>

struct B
{
enum Eb
{
ebAsd1, ebAsd2, ebAsd3,
};

enum{ unnamedEnumerator };

enum class Fruit2
{
Apple,
Strawberry,
Orange,
Cherry
};

};

enum class Fruit
{
Apple,
Strawberry,
Orange,
Cherry
};

enum class Color
{
Red,
Orange,
Green
};

int main()
{

   return 0;
}


Fruit2 is broken, their values end up in the higher scope, which is incorrect. This workaround (patch) should not trigger when enum class.

To which CC guru do we send this ?

oBFusCATed

Quote from: killerbot on January 21, 2013, 08:18:46 PM
To which CC guru do we send this ?
Ollydbg he is the only one:)

Keep in mind that we don't claim support for C++11, also the normal C++98/C style enums are way more common.
The old code worked by chance I suppose. So if you ask me and there are no other blocker it is better.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

p2rkw

How about this:
Index: src/plugins/codecompletion/parser/parserthread.cpp
===================================================================
--- src/plugins/codecompletion/parser/parserthread.cpp (wersja 8785)
+++ src/plugins/codecompletion/parser/parserthread.cpp (kopia robocza)
@@ -2263,10 +2282,13 @@
     // enums have the following rough definition:
     // enum [xxx] { type1 name1 [= 1][, [type2 name2 [= 2]]] };
     bool isUnnamed = false;
+    bool isEnumClass = false;
     int lineNr = m_Tokenizer.GetLineNumber();
     wxString token = m_Tokenizer.GetToken();
-    if (token == ParserConsts::kw_class)
+    if (token == ParserConsts::kw_class){
         token = m_Tokenizer.GetToken();
+        isEnumClass = true;
+    }
     if (token.IsEmpty())
         return;

@@ -2357,7 +2379,8 @@
             {
                 Token* lastParent = m_LastParent;
                 m_LastParent = newEnum;
-                DoAddToken(tkEnumerator, token, m_Tokenizer.GetLineNumber());
+                Token* enumerator = DoAddToken(tkEnumerator, token, m_Tokenizer.GetLineNumber());
+                enumerator->m_Scope = isEnumClass ? tsPrivate : tsPublic;
                 m_LastParent = lastParent;
             }
             if (peek==ParserConsts::colon)
Index: src/plugins/codecompletion/nativeparser_base.h
===================================================================
--- src/plugins/codecompletion/nativeparser_base.h (wersja 8785)
+++ src/plugins/codecompletion/nativeparser_base.h (kopia robocza)
@@ -396,14 +396,16 @@
     // for GenerateResultSet()
     bool AddChildrenOfUnnamed(TokenTree* tree, const Token* parent, TokenIdxSet& result)
     {
-        if (parent->m_TokenKind == tkClass && parent->m_Name.StartsWith(g_UnnamedSymbol))
+        if (((parent->m_TokenKind & (tkClass | tkEnum)) != 0)
+            && parent->m_Name.StartsWith(g_UnnamedSymbol))
         {
             // add all its children
             for (TokenIdxSet::const_iterator it = parent->m_Children.begin();
                                              it != parent->m_Children.end(); ++it)
             {
                 Token* tokenChild = tree->at(*it);
-                if (tokenChild)
+                if (tokenChild &&
+                    (parent->m_TokenKind == tkClass || tokenChild->m_Scope != tsPrivate))
                     result.insert(*it);
             }

@@ -411,7 +413,25 @@
         }
         return false;
     }
+   
+    bool AddChildrenOfEnum(TokenTree* tree, const Token* parent, TokenIdxSet& result)
+    {
+        if (parent->m_TokenKind == tkEnum)
+        {
+            // add all its children
+            for (TokenIdxSet::const_iterator it = parent->m_Children.begin();
+                                             it != parent->m_Children.end(); ++it)
+            {
+                Token* tokenChild = tree->at(*it);
+                if (tokenChild && tokenChild->m_Scope != tsPrivate)
+                    result.insert(*it);
+            }

+            return true;
+        }
+        return false;
+    }
+
     // for GenerateResultSet()
     bool MatchText(const wxString& text, const wxString& target, bool caseSens, bool isPrefix)
     {
Index: src/plugins/codecompletion/nativeparser_base.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser_base.cpp (wersja 8785)
+++ src/plugins/codecompletion/nativeparser_base.cpp (kopia robocza)
@@ -1318,7 +1318,11 @@
                 if (!token)
                     continue;
                 if ( !AddChildrenOfUnnamed(tree, token, result) )
+                {
                     result.insert(*it);
+                    AddChildrenOfEnum(tree, token, result);
+                }
+                   
             }

             tree->RecalcInheritanceChain(parent);
@@ -1334,7 +1338,10 @@
                     if (!token)
                         continue;
                     if ( !AddChildrenOfUnnamed(tree, token, result) )
+                    {
                         result.insert(*it2);
+                        AddChildrenOfEnum(tree, token, result);
+                    }
                 }
             }
         }


oBFusCATed

p2rkw: Can you try to add some tests? See Morten's comment.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

killerbot

YES : this updated patch seems to work correctly    ;D

MortenMacFly

Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]

Martin K.

Hi,

whats about typedef union and unnamed members? It seems that something like this:

typedef union
{
    unsigned short Value;

    struct
    {
        unsigned short Var1:6;
        unsigned short Var2:10;
    };
}
SOME_ID, *PSOME_ID;

could not be found.

MortenMacFly

Quote from: Martin K. on January 22, 2013, 11:01:54 AM
whats about typedef union and unnamed members? It seems that something like this:
...added another test case to SVN (and fixed some warnings).
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: [url="https://www.codeblocks.org/docs/main_codeblocks_en.html"]https://www.codeblocks.org/docs/main_codeblocks_en.html[/url]
C::B FAQ: [url="https://wiki.codeblocks.org/index.php?title=FAQ"]https://wiki.codeblocks.org/index.php?title=FAQ[/url]

ollydbg

Quote from: Martin K. on January 22, 2013, 11:01:54 AM
Hi,

whats about typedef union and unnamed members? It seems that something like this:

typedef union
{
    unsigned short Value;

    struct
    {
        unsigned short Var1:6;
        unsigned short Var2:10;
    };
}
SOME_ID, *PSOME_ID;

could not be found.

I think this bug should be discussed in another thread :), and let's focus on handling emun declaration in this thread.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.