Print Page - logic error on the SupportsCurrentPlatform() related code

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => CodeCompletion redesign => Topic started by: ollydbg on April 08, 2015, 09:37:38 am

Title: logic error on the SupportsCurrentPlatform() related code
Post by: ollydbg on April 08, 2015, 09:37:38 am

@Morten
I see the recent changes in rev 10189 and rev 10194, debugged a little, I think the predefined macros in our CCDebugDialog is wrong.

Question:

Code

bool NativeParser::AddCompilerPredefinedMacrosGCC(const wxString& compilerId, cbProject* project, wxString& defs, ParserBase* parser)
{
    Compiler* compiler = CompilerFactory::GetCompiler(compilerId);
    if (!compiler)
        return false;

    if (   !parser->Options().platformCheck
        || (parser->Options().platformCheck && compiler->SupportsCurrentPlatform()) )
    {
        TRACE(_T("NativeParser::AddCompilerPredefinedMacrosGCC: Not supported on current platform!"));
        return false;
    }

Do you think there should be a ! after &&.

Also, if user don't want that platform check, do we need to "return false;"? I think in this case, we should run the old logic, or maybe, we need to remove the line:

Code

!parser->Options().platformCheck

Title: Re: logic error on the SupportsCurrentPlatform() related code
Post by: MortenMacFly on April 08, 2015, 09:46:12 am

Looks like that on second thoughts... maybe I was drunk. :-\

Will you take the action?

Title: Re: logic error on the SupportsCurrentPlatform() related code
Post by: ollydbg on April 08, 2015, 09:51:25 am

Quote from: MortenMacFly on April 08, 2015, 09:46:12 am

Looks like that on second thoughts... maybe I was drunk. :-\

Will you take the action?

OK.
I will fix it soon.

Title: Re: logic error on the SupportsCurrentPlatform() related code
Post by: ollydbg on April 08, 2015, 10:32:31 am

Fixed in trunk(r10198)

Title: Re: logic error on the SupportsCurrentPlatform() related code
Post by: Huki on April 08, 2015, 02:44:58 pm

Oh yes, I discovered this error yesterday and added the same fix as in r10198.
Btw, on Linux there is another problem with backslash in: "\\bin\\", this has to be taken care too. Here is the entire patch:

Code

From dc6aa186413beff51b7a95f4010cd03cc1390c06 Mon Sep 17 00:00:00 2001
From: huki <gk7huki@gmail.com>
Date: Wed, 8 Apr 2015 02:02:25 +0530
Subject: CC: fix compiler predefined macros collection

---
 src/plugins/codecompletion/nativeparser.cpp | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/plugins/codecompletion/nativeparser.cpp b/src/plugins/codecompletion/nativeparser.cpp
index bf9113d..e17ede1 100644
--- a/src/plugins/codecompletion/nativeparser.cpp
+++ b/src/plugins/codecompletion/nativeparser.cpp
@@ -2000,8 +2000,7 @@ bool NativeParser::AddCompilerPredefinedMacrosGCC(const wxString& compilerId, cb
     if (!compiler)
         return false;
 
-    if (   !parser->Options().platformCheck
-        || (parser->Options().platformCheck && compiler->SupportsCurrentPlatform()) )
+    if ( parser->Options().platformCheck && !compiler->SupportsCurrentPlatform() )
     {
         TRACE(_T("NativeParser::AddCompilerPredefinedMacrosGCC: Not supported on current platform!"));
         return false;
@@ -2009,7 +2008,11 @@ bool NativeParser::AddCompilerPredefinedMacrosGCC(const wxString& compilerId, cb
 
     wxString masterPath = compiler->GetMasterPath();
     Manager::Get()->GetMacrosManager()->ReplaceMacros(masterPath);
+#ifdef __WXMSW__
     const wxString cpp_compiler = masterPath + _T("\\bin\\") + compiler->GetPrograms().CPP;
+#else
+    const wxString cpp_compiler = masterPath + _T("/bin/") + compiler->GetPrograms().CPP;
+#endif
     if ( !wxFileName::FileExists(cpp_compiler) )
         return false;
 
@@ -2116,8 +2119,7 @@ bool NativeParser::AddCompilerPredefinedMacrosVC(const wxString& compilerId, wxS
     if (!compiler)
         return false;
 
-    if (   !parser->Options().platformCheck
-        || (parser->Options().platformCheck && compiler->SupportsCurrentPlatform()) )
+    if ( parser->Options().platformCheck && !compiler->SupportsCurrentPlatform() )
     {
         TRACE(_T("NativeParser::AddCompilerPredefinedMacrosVC: Not supported on current platform!"));
         return false;
@@ -2125,7 +2127,11 @@ bool NativeParser::AddCompilerPredefinedMacrosVC(const wxString& compilerId, wxS
 
     wxString masterPath = compiler->GetMasterPath();
     Manager::Get()->GetMacrosManager()->ReplaceMacros(masterPath);
+#ifdef __WXMSW__
     const wxString c_compiler = masterPath + _T("\\bin\\") + compiler->GetPrograms().C;
+#else
+    const wxString c_compiler = masterPath + _T("/bin/") + compiler->GetPrograms().C;
+#endif
     if ( !wxFileName::FileExists(c_compiler) )
         return false;
 
-- 
1.9.1

Title: Re: logic error on the SupportsCurrentPlatform() related code
Post by: ollydbg on April 08, 2015, 03:38:50 pm

Hi, Huki, thanks, before commit the fix to trunk, I just want to ask a simple question:

Code

@@ -2009,7 +2008,11 @@ bool NativeParser::AddCompilerPredefinedMacrosGCC(const wxString& compilerId, cb
 
     wxString masterPath = compiler->GetMasterPath();
     Manager::Get()->GetMacrosManager()->ReplaceMacros(masterPath);
+#ifdef __WXMSW__
     const wxString cpp_compiler = masterPath + _T("\\bin\\") + compiler->GetPrograms().CPP;
+#else
+    const wxString cpp_compiler = masterPath + _T("/bin/") + compiler->GetPrograms().CPP;
+#endif
     if ( !wxFileName::FileExists(cpp_compiler) )
         return false;

What was the problem if we don't have the above patch applied under Linux? Will wxFileName::FileExists() function call fail? If this does fail, then I believe this is a very long time bug. I don't have a Linux system at hand, so I can't verify myself. :)

Title: Re: logic error on the SupportsCurrentPlatform() related code
Post by: Huki on April 08, 2015, 05:10:15 pm

Quote from: ollydbg on April 08, 2015, 03:38:50 pm

What was the problem if we don't have the above patch applied under Linux? Will wxFileName::FileExists() function call fail? If this does fail, then I believe this is a very long time bug. I don't have a Linux system at hand, so I can't verify myself. :)

Yes, the wxFileName::FileExists() call fails. I just tried skipping this check and keep the backward slash to see what happens: then the wxExecute does not report a failure, but no defines are collected (the defines list is empty). So it seems the wxExecute actually fails but it doesn't get reported.
By using forward slash for "/bin/" the defines are collected properly.

Title: Re: logic error on the SupportsCurrentPlatform() related code
Post by: ollydbg on April 09, 2015, 04:12:23 pm

Quote from: Huki on April 08, 2015, 05:10:15 pm

Quote from: ollydbg on April 08, 2015, 03:38:50 pm
What was the problem if we don't have the above patch applied under Linux? Will wxFileName::FileExists() function call fail? If this does fail, then I believe this is a very long time bug. I don't have a Linux system at hand, so I can't verify myself. :)
Yes, the wxFileName::FileExists() call fails. I just tried skipping this check and keep the backward slash to see what happens: then the wxExecute does not report a failure, but no defines are collected (the defines list is empty). So it seems the wxExecute actually fails but it doesn't get reported.
By using forward slash for "/bin/" the defines are collected properly.

Thanks, and it is in trunk now.

Code::Blocks Forums

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => CodeCompletion redesign => Topic started by: ollydbg on April 08, 2015, 09:37:38 am