Compiler code generating error

Dominik Egert  

Hello,

I have an issue with reference variables as parameter to a function. It is not easy to reconstruct, therefore I will do my best to try to explain the issue with sample code. It might be, the code is not compliant, but for explaining the issue, it should give an idea on how to reconstruct the error.

The issue happens as follows

- A structure used as type for a dynamic array.

- A function going throu the array and calling a function, passing the index of the element from the array to process some values.

- The processing function (overloaded 2 times + base-version) accesses the arrays structure elements and passes them down to one of the overloaded functions as reference.

- The sub-function called assigns values to the elements passed by reference.

The assigned values are defective due to some addressing-issue brought in by the compiler.


Here is the code:

// The structure

// Structure holding infos about positions and orders
struct order_position_info
{
    ulong       ticket;
    datetime    open_time; 
    datetime    update_time; 
    ulong       age_secs;
    double      open_price; 
    double      stop_loss; 
    double      initial_stop_loss;
    double      take_profit;
    double      initial_take_profit;
    double      curren_price; 
    double      profit;
    int         profit_ticks;
    double      volume; 
    double      initial_volume;
    double      cancel_level;
    bool        type_sell;
    string      symbol;
};


void parent_loop()
{
        // Local init

                const int elements = ArraySize(data::position_list);


/*****************************************************************************/
// [...] -> Some code is spared here
// data::position_list[] is a global array, 
//                                               maintained by the missing code of this function.
//
/*****************************************************************************/


    // Check existing positions

        for(; (cnt < elements) && !_StopFlag; cnt++)
        {
            // Reset last error
            ResetLastError();

            // Load all relevant positions data
            PositionSelectByTicket(tickets[cnt]);
            sl      = PositionGetDouble(POSITION_SL);
            tp      = PositionGetDouble(POSITION_TP);
            vol     = PositionGetDouble(POSITION_VOLUME);
            symbol  = PositionGetString(POSITION_SYMBOL);

            // Update meta data
            data::position_list[cnt].open_time      = (datetime)PositionGetInteger(POSITION_TIME);
            data::position_list[cnt].update_time    = (datetime)PositionGetInteger(POSITION_TIME_UPDATE);
            data::position_list[cnt].age_secs       = TimeCurrent() - data::position_list[cnt].open_time;
            data::position_list[cnt].type_sell      = (PositionGetInteger(POSITION_TYPE) == POSITION_TYPE_SELL);
            data::position_list[cnt].symbol         = symbol;
            data::position_list[cnt].open_price     = PositionGetDouble(POSITION_PRICE_OPEN);

            // Read current geometry
            data::position_list[cnt].stop_loss      = sl;
            data::position_list[cnt].take_profit    = tp;
            data::position_list[cnt].curren_price   = PositionGetDouble(POSITION_PRICE_CURRENT);
            data::position_list[cnt].profit         = PositionGetDouble(POSITION_PROFIT);
            data::position_list[cnt].volume         = vol;

            // Set ticket number
            data::position_list[cnt].ticket         = tickets[cnt];


            
/*****************************************************************************/
// Here the overloaded function gets called
// At this point all needed values have been initialized.
// Access to the structure is working, checked with printf()
//
                        // Read initial geometry (tp/sl/vol)
                        pos_initial_geometry(cnt);
/*****************************************************************************/




            // Calculate profit in ticks
            data::position_list[cnt].profit_ticks   = (int)(((data::position_list[cnt].type_sell) ? (data::position_list[cnt].open_price - PositionGetDouble(POSITION_PRICE_CURRENT)) : (PositionGetDouble(POSITION_PRICE_CURRENT) - data::position_list[cnt].open_price) ) * MathPow(10, SymbolInfoInteger(data::position_list[cnt].symbol, SYMBOL_DIGITS)));

            // Call PnL handler
            err_counter         += (OnFloatingPnL(cnt) & 0x01);
            
            // Update tp, sl
            if( (sl != data::position_list[cnt].stop_loss) || (tp != data::position_list[cnt].take_profit) )
            { err_counter       += (!pos_change_sltp(data::position_list[cnt].stop_loss, data::position_list[cnt].take_profit) & 0x01);  }

            // Update position volume
            if(vol - data::position_list[cnt].volume > NULL) 
            { err_counter       += ((!ord_close_ticket(NULL, vol - data::position_list[cnt].volume, "Partial close")) & 0x01); }

            // Add volume to a netting position
            // Open new position on different symbol
            else if( ((netting) && (vol - data::position_list[cnt].volume < NULL)) || (data::position_list[cnt].symbol != symbol) )
            { 
                err_counter     += ((!ord_open( NULL,                                                                                               // Open price
                                                data::position_list[cnt].type_sell,                                                                 // Direction
                                                data::position_list[cnt].volume - ((data::position_list[cnt].symbol != symbol) ? NULL : vol),       // Volume
                                                data::position_list[cnt].stop_loss,                                                                 // Stop loss
                                                data::position_list[cnt].take_profit,                                                               // Take profit
                                                NULL,                                                                                               // Cancel level (pending orders)
                                                data::position_list[cnt].symbol)) & 0x01); 
            }
        }
}




/**********************************************************************************************************************************************************************************************/
//
//      This function gets called first and then calls the overloaded function below, which in turn calls the last overload
//

// Define comment tokens
#define _ORD_CANCEL_LVL_ID  "C:"
#define _ORD_STOP_LOSS_ID   "S:"
#define _ORD_TAKE_PROFIT_ID "T:"
#define _ORD_VOLUME_ID      "V:"
#define _ORD_COMMENT_SEPARATOR ";"



//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const int pos_id)
{
    // Preinit
    data::position_list[pos_id].initial_stop_loss = data::position_list[pos_id].open_price;
    data::position_list[pos_id].initial_take_profit = data::position_list[pos_id].open_price;

    // Return   
    return(pos_initial_geometry(data::position_list[pos_id].ticket,
                                data::position_list[pos_id].initial_stop_loss,
                                data::position_list[pos_id].initial_take_profit,
                                data::position_list[pos_id].initial_volume));
}


//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const ulong ticket, double& sl, double& tp, double& vol)
{
    // Local init
        const double digits     = MathPow(10.0, SymbolInfoInteger(PositionGetString(POSITION_SYMBOL), SYMBOL_DIGITS));
        long sl_ticks           = NULL;
        long tp_ticks           = NULL;

    // Extract values
    if(!pos_initial_geometry(ticket, sl_ticks, tp_ticks, vol))
    { return(false); }

    // Calculate absolute values
    sl += ((double)sl_ticks / digits);
    tp += ((double)tp_ticks / digits);
    
    // Return
    return(true);
}


//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const ulong ticket, long& sl_ticks, long& tp_ticks, double& vol)
{
    // Local init
    sl_ticks    = NULL;
    tp_ticks    = NULL;
    vol         = NULL;
    
    // Select position
    if((ticket > NULL) && !PositionSelectByTicket(ticket) && OnError())
    { return(false); }

    // Get comment string
    //const string comment = PositionGetString(POSITION_COMMENT);
        
        // DEBUG init value
        const string comment = "S:65;T:163;V:0.12;";


    // Check comment
    const int   sl_id_len   = StringLen(_ORD_STOP_LOSS_ID);
    const int   tp_id_len   = StringLen(_ORD_TAKE_PROFIT_ID);
    const int   vol_id_len  = StringLen(_ORD_VOLUME_ID);
    const int   sl_idx      = StringFind(comment, _ORD_STOP_LOSS_ID) + sl_id_len;
    const int   sl_len      = StringFind(comment, _ORD_COMMENT_SEPARATOR, sl_idx) - (sl_idx);
    const int   tp_idx      = StringFind(comment, _ORD_TAKE_PROFIT_ID) + tp_id_len;
    const int   tp_len      = StringFind(comment, _ORD_COMMENT_SEPARATOR, tp_idx) - (tp_idx);
    const int   vol_idx     = StringFind(comment, _ORD_VOLUME_ID) + vol_id_len;
    const int   vol_len     = StringFind(comment, _ORD_COMMENT_SEPARATOR, vol_idx) - (vol_idx);
        const bool  bShort          = (PositionGetInteger(POSITION_TYPE) == POSITION_TYPE_SELL);

    // Évaluate    
    if( (sl_idx == -1)
     || (tp_idx == -1)
     || (vol_idx == -1) )
    { return(false); }



        // DEBUG verify values
                // Extract values
                string sl_test = StringSubstr(comment, sl_idx, sl_len);
                string tp_test = StringSubstr(comment, tp_idx, tp_len);
                string vol_tst = StringSubstr(comment, vol_idx, vol_len);
                
                
                long test_sl = (int)StringToInteger(sl_test);
                long test_tp = (int)StringToInteger(tp_test);
                double tst_vol = StringToDouble(vol_tst);

/*****************************************************************************/
// After this string extraction the values assigned are
// test_sl = 65
// test_tp = 163
// tst_vol = 0.119999999999999996
/*****************************************************************************/
                

                long sl_dir = (-1 + (2 * ((bShort) & 0x01)));
                long tp_dir = (-1 + (2 * ((!bShort) & 0x01)));
                
/*****************************************************************************/
// sl_ticks is at the moment = NULL;
// tp_ticks is at the moment = NULL;
// vol is at the moment = NULL;
//
// initialized in parent function

                sl_ticks = test_sl;             
                tp_ticks = test_tp;
                vol = tst_vol;

// After this assignment the values are 
// sl_ticks = 65
// tp_ticks = 163
// vol = 1.9522361275299495E-314
                
                sl_ticks *= sl_dir;
                tp_ticks *= tp_dir;

// After this operation the values are
// sl_ticks = 4294967231
// tp_ticks = 163
/*****************************************************************************/



/*****************************************************************************/
// I suppose everyone can agree on the fact, that these variables 
// should get assigned the value from the multiply operation results
// ... but they dont. - They get garbage data inserted

                long test_sl_ticks = test_sl * sl_dir;
                long test_tp_ticks = test_tp * tp_dir;

// After this operation the values are
// test_sl_ticks = 279172874175
// test_tp_ticks = 163
/*****************************************************************************/


/*****************************************************************************/
// Also this does not result in valid values....    
                sl_ticks = test_sl_ticks;
                tp_ticks = test_tp_ticks;
                vol = tst_vol;
                
// After this operation the values are
// sl_ticks = 4294967231
// tp_ticks = 163
// vol = 1.9522361275299495E-314
/*****************************************************************************/


        // Assign values
    sl_ticks = ((long)StringToInteger(StringSubstr(comment, sl_idx, sl_len))) * ((long)(-1 + (2 * ((bShort) & 0x01))));
    tp_ticks = ((long)StringToInteger(StringSubstr(comment, tp_idx, tp_len))) * ((long)(-1 + (2 * ((!bShort) & 0x01))));
    vol = StringToDouble(StringSubstr(comment, vol_idx, vol_len));
    
/*****************************************************************************/
// After this operation the result is the same
/*****************************************************************************/

    // Return
    return(true);
}




I suspect the compiler casts the value addresses as 32 bit values and therefore mixes the assignment and typedefs up.

At least it seems to me to be so, since the following code does work as expected:


//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const int pos_id)
{
    // Local init
          double    isl     = data::position_list[pos_id].open_price;
          double    itp     = data::position_list[pos_id].open_price;
          double    ivol    = NULL;
    const bool      retval  = pos_initial_geometry((ulong)data::position_list[pos_id].ticket, isl, itp, ivol);

    // Assign values
    data::position_list[pos_id].initial_stop_loss = isl;
    data::position_list[pos_id].initial_take_profit = itp;
    data::position_list[pos_id].initial_volume = ivol;

    // Return    
    return(retval);
}


//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(ulong ticket, double& sl, double& tp, double& vol)
{
    // Local init
        const double    price_digit_f   = MathPow(10.0, SymbolInfoInteger(PositionGetString(POSITION_SYMBOL), SYMBOL_DIGITS));
              int       sl_ticks                = NULL;
              int       tp_ticks                = NULL;
              int       sub_vol         = NULL;

    // Extract values
    if(!pos_initial_geometry(ticket, sl_ticks, tp_ticks, vol))
    { return(false); }

    // Calculate absolute values
    sl += (sl_ticks / price_digit_f);
    tp += (tp_ticks / price_digit_f);
    
    // Return
    return(true);
}


//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(ulong ticket, int& sl_ticks, int& tp_ticks, double& vol)
{
    // Local init
    sl_ticks    = NULL;
    tp_ticks    = NULL;
    vol         = NULL;
    
    // Select position
    if((ticket > NULL) && !PositionSelectByTicket(ticket) && OnError())
    { return(false); }

    // Get comment string
    const string comment = PositionGetString(POSITION_COMMENT);

    // Check comment
    const int   sl_id_len   = StringLen(_ORD_STOP_LOSS_ID);
    const int   tp_id_len   = StringLen(_ORD_TAKE_PROFIT_ID);
    const int   vol_id_len  = StringLen(_ORD_VOLUME_ID);
    const int   sl_idx      = StringFind(comment, _ORD_STOP_LOSS_ID) + sl_id_len;
    const int   sl_len      = StringFind(comment, _ORD_COMMENT_SEPARATOR, sl_idx) - (sl_idx);
    const int   tp_idx      = StringFind(comment, _ORD_TAKE_PROFIT_ID) + tp_id_len;
    const int   tp_len      = StringFind(comment, _ORD_COMMENT_SEPARATOR, tp_idx) - (tp_idx);
    const int   vol_idx     = StringFind(comment, _ORD_VOLUME_ID) + vol_id_len;
    const int   vol_len     = StringFind(comment, _ORD_COMMENT_SEPARATOR, vol_idx) - (vol_idx);
        const bool  bShort          = (PositionGetInteger(POSITION_TYPE) == POSITION_TYPE_SELL);

    // Évaluate    
    if( (sl_idx == -1)
     || (tp_idx == -1)
     || (vol_idx == -1) )
    { return(false); }

    // Extract values
    sl_ticks = ((int)StringToInteger(StringSubstr(comment, sl_idx, sl_len))) * ((int)(-1 + (2 * ((bShort) & 0x01))));
    tp_ticks = ((int)StringToInteger(StringSubstr(comment, tp_idx, tp_len))) * ((int)(-1 + (2 * ((!bShort) & 0x01))));
    vol = StringToDouble(StringSubstr(comment, vol_idx, vol_len));
    
    // Return
    return(true);
}


These functions are my current actual implementation.

lippmaje  
   bool bShort=false;
   long sl_dir = (-1 + (2 * ((bShort) & 0x01)));
   Print("dir: ",sl_dir); // dir: 4294967295

fix:

long sl_dir = (-1 + (2 * ((long)(bShort) & 0x01)));

or just

long sl_dir = (bShort ? 1 : -1);
or store it in an int.
Dominik Egert  
lippmaje:

fix:

or just

or store it in an int.

Thank you for your reply.

I think you missed the issue.

The code part you refere to is only for debugging. At the end of the function, the value assignment is done.

I have myself already tried to fix it using 32-bit variables, but it does not fix the issue.


But, you have missed the source of the problem as far as I can tell. - The issue is sourced in the addressing of reference-variables itself. So the signature of the function is not consistent.

To give you an idea of what I mean, here a short description:

A structure as type for a dynamic array, located on the global memory space of the program, framed by a namespace, namely here "data".

A selected "entry" from the array, a member from the underlying structure passed as reference to a function, this function in turn passes its given reference down to another function, also as reference.

Down here in the second "sub"-function the issue occures by assigning a value, giving "scrambled eggs" as a result.

These "scrambled eggs" are consistently passed back up, so the memory in the global space gets changed as intended, but not with the correct value.


So the issue is stil lthere and your suggestion does not solve the issue. - I have posted code, which in fact works, so as long as I declare the variable which gets passed to a function as reference, on the stack of the calling function, it works. But this cannot be done in "inception". Meaning if I pass down the by reference given variable, it gets mixed up somehow. (It seems to me as if the datatype gets casted to signed int, but only "virtually", since the compiler does not report an assignment cast problem.)

Please see the code which I have posted as working.

Documentation on MQL5: Constants, Enumerations and Structures / Named Constants / Predefined Macro Substitutions
Documentation on MQL5: Constants, Enumerations and Structures / Named Constants / Predefined Macro Substitutions
  • www.mql5.com
Predefined Macro Substitutions - Named Constants - Constants, Enumerations and Structures - MQL5 Reference - Reference on algorithmic/automated trading language for MetaTrader 5
Dominik Egert  
lippmaje:

fix:

or just

or store it in an int.
long sl_dir = (-1 + (2 * ((long)(bShort) & 0x01)));

I would like to explain whats going on here. And why the cast operator (long) is not needed.

a boolen variable is stored as a 8 bit variable. See doc: https://www.mql5.com/en/docs/basis/types/integer/boolconst

So when "AND" operated a constant hex value (here 0x01, which in turn gets interpreted as unsigned char/short/int/long) with a boolen value, there is no casting operator needed. Thus, the operation "bool" & "uint" will result on the CPU as ulong & ulong, since all registers on a 64 Bit CPU are 64 bit wide.

This is in fact the fastest way of checking a boolen value.

Si if you begin to cast the values, you will (if not remove-optimized by compiler) introduce another operation on the value, which is simply not needed.


Contrary to your suggestion, which might seem logical, but is wrong:

long sl_dir = (bShort ? 1 : -1);

this can be written as:

long sl_dir = 1;

if(bShort)
{ sl_dir *= -1; }

and is effectiveley the same on the CPU, is significantly slower in execution.

This is due to the behaviour of the code-path the CPU executes. 

Your suggestion will introduce a conditional jump in execution, which is not desirable. In fact the worst situation for a CPU.

For further readings, I suggest this article:

http://igoro.com/archive/fast-and-slow-if-statements-branch-prediction-in-modern-processors/

Documentation on MQL5: Language Basics / Data Types / Integer Types / Bool Type
Documentation on MQL5: Language Basics / Data Types / Integer Types / Bool Type
  • www.mql5.com
Bool Type - Integer Types - Data Types - Language Basics - MQL5 Reference - Reference on algorithmic/automated trading language for MetaTrader 5
lippmaje  
Dominik Egert:

I would like to explain whats going on here. And why the cast operator (long) is not needed.

a boolen variable is stored as a 8 bit variable. See doc: https://www.mql5.com/en/docs/basis/types/integer/boolconst

So when "AND" operated a constant hex value (here 0x01, which in turn gets interpreted as unsigned char/short/int/long) with a boolen value, there is no casting operator needed. Thus, the operation "bool" & "uint" will result on the CPU as ulong & ulong, since all registers on a 64 Bit CPU are 64 bit wide.

This is in fact the fastest way of checking a boolen value.

Si if you begin to cast the values, you will (if not remove-optimized by compiler) introduce another operation on the value, which is simply not needed.

But your operation results in 4294967295 and not -1 as you expected. This is what I get in MT5 as well as MT4, no matter what you might infer from the documentation, which is sometimes a bit misleading or outdated.

Dominik Egert  
lippmaje:

But your operation results in 4294967295 and not -1 as you expected. This is what I get in MT5 as well as MT4, no matter what you might infer from the documentation, which is sometimes a bit misleading or outdated.

Thank you again for pointing this out.

Still I have issues to communicate the real problem, as it is hidden in the resulting binary, not the high-level-language code written here.

You keep refering to code which is only for debugging inserted. - The issue is solved by using a variable located on a stack of a function, if the variable is from the global memory space, it gets mixed up. / Or maybe it is due to a problem in the type-safe operation from the value given by the structure.

This can be reproduced, btw, if you use my original code and go throu in the debugger. (SOme variables need to be added to the first function, and some stuff needs to be removed, since it is not existent in the posted code, but the issue stays the same. - Please try it yourself.

And BTW, it does not explain the behaviour done on the double-value. Which is also wrong.

Documentation on MQL5: Constants, Enumerations and Structures / Named Constants / Predefined Macro Substitutions
Documentation on MQL5: Constants, Enumerations and Structures / Named Constants / Predefined Macro Substitutions
  • www.mql5.com
Predefined Macro Substitutions - Named Constants - Constants, Enumerations and Structures - MQL5 Reference - Reference on algorithmic/automated trading language for MetaTrader 5
lippmaje  

In the comments of your code you wrote this:

// I suppose everyone can agree on the fact, that these variables 
// should get assigned the value from the multiply operation results
// ... but they dont. - They get garbage data inserted

                long test_sl_ticks = test_sl * sl_dir;
                long test_tp_ticks = test_tp * tp_dir;

// After this operation the values are
// test_sl_ticks = 279172874175
// test_tp_ticks = 163

So you were pointing out a "garbage" value, but it isn't. It is the result of multiplying test_sl (65, correctly derived from the order comment) with sl_dir (4294967295, erroneous).

If there is anything else that appears to be buggy, I suggest to fix this example code, strip the order logic and post it again.

There's a thread in the Russian forum where you can submit any error reports.

Dominik Egert  
lippmaje:

In the comments of your code you wrote this:

So you were pointing out a "garbage" value, but it isn't. It is the result of multiplying test_sl (65, correctly derived from the order comment) with sl_dir (4294967295, erroneous).

If there is anything else that appears to be buggy, I suggest to fix this example code, strip the order logic and post it again.

There's a thread in the Russian forum where you can submit any error reports.

Thank you. Yes, I see what you are aiming at. - It seems to me, a demo code is needed to show the issue.

//+------------------------------------------------------------------+
//|                                                   mql_market.mq5 |
//|             Copyright 2020, Freie Netze UG (haftunbgsbeschränkt) |
//|                                      https://www.freie-netze.de/ |
//+------------------------------------------------------------------+
#property copyright "Copyright 2020, Freie Netze UG (haftunbgsbeschränkt)"
#property link      "https://www.freie-netze.de/"
#property version   "1.00"




//+------------------------------------------------------------------+
//| Expert initialization function                                   |
//+------------------------------------------------------------------+
int OnInit()
   {
//---
    parent_loop();
    
    return(INIT_SUCCEEDED);
   }


//+------------------------------------------------------------------+
//| Expert deinitialization function                                 |
//+------------------------------------------------------------------+
void OnDeinit(const int reason)
   {
//---

   }


//+------------------------------------------------------------------+
//| Expert tick function                                             |
//+------------------------------------------------------------------+
void OnTick()
   {
//---

   }
//+------------------------------------------------------------------+

/***********************************************************************************************************************************************************/


// Define comment tokens
#define _ORD_CANCEL_LVL_ID  "C:"
#define _ORD_STOP_LOSS_ID   "S:"
#define _ORD_TAKE_PROFIT_ID "T:"
#define _ORD_VOLUME_ID      "V:"
#define _ORD_COMMENT_SEPARATOR ";"



// Structure holding infos about positions and orders
struct order_position_info
{
    ulong       ticket;
    datetime    open_time; 
    datetime    update_time; 
    ulong       age_secs;
    double      open_price; 
    double      stop_loss; 
    double      initial_stop_loss;
    double      take_profit;
    double      initial_take_profit;
    double      curren_price; 
    double      profit;
    double      volume; 
    double      initial_volume;
    double      cancel_level;
    int         profit_ticks;
    bool        type_sell;
    string      symbol;
};

namespace data
{

        static order_position_info position_list[];

};



void parent_loop()
{
        // Debug init array for demo 
        
                ArrayResize(data::position_list, 1);
                ZeroMemory(data::position_list);


        // Local init

                const int elements = ArraySize(data::position_list);


    // Check existing positions

        for(int cnt = NULL; (cnt < elements) && !_StopFlag; cnt++)
        {
            data::position_list[cnt].open_price     = 1.21345;
            data::position_list[cnt].type_sell      = true;

            
/*****************************************************************************/
// Here the overloaded function gets called
// At this point all needed values have been initialized.
// Access to the structure is working, checked with printf()
//
                        // Read initial geometry (tp/sl/vol)
                        pos_initial_geometry(cnt);
/*****************************************************************************/


        }
}




/**********************************************************************************************************************************************************************************************/
//
//      This function gets called first and then calls the overloaded function below, which in turn calls the last overload
//


//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const int pos_id)
{
    // Preinit
    data::position_list[pos_id].initial_stop_loss = data::position_list[pos_id].open_price;
    data::position_list[pos_id].initial_take_profit = data::position_list[pos_id].open_price;

    // Return   
    return(pos_initial_geometry(data::position_list[pos_id].ticket,
                                data::position_list[pos_id].initial_stop_loss,
                                data::position_list[pos_id].initial_take_profit,
                                data::position_list[pos_id].initial_volume));
}


//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const ulong ticket, double& sl, double& tp, double& vol)
{
    // Local init
        const double digits     = MathPow(10.0, SymbolInfoInteger(PositionGetString(POSITION_SYMBOL), SYMBOL_DIGITS));
        long sl_ticks           = NULL;
        long tp_ticks           = NULL;

    // Extract values
    if(!pos_initial_geometry(ticket, sl_ticks, tp_ticks, vol))
    { return(false); }

    // Calculate absolute values
    sl += ((double)sl_ticks / digits);
    tp += ((double)tp_ticks / digits);
    
    // Return
    return(true);
}


//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const ulong ticket, long& sl_ticks, long& tp_ticks, double& vol)
{
    // Local init
    sl_ticks    = NULL;
    tp_ticks    = NULL;
    vol         = NULL;
    
        // DEBUG init value
        const string comment = "S:65;T:163;V:0.12;";


    // Check comment
    const int   sl_id_len   = StringLen(_ORD_STOP_LOSS_ID);
    const int   tp_id_len   = StringLen(_ORD_TAKE_PROFIT_ID);
    const int   vol_id_len  = StringLen(_ORD_VOLUME_ID);
    const int   sl_idx      = StringFind(comment, _ORD_STOP_LOSS_ID) + sl_id_len;
    const int   sl_len      = StringFind(comment, _ORD_COMMENT_SEPARATOR, sl_idx) - (sl_idx);
    const int   tp_idx      = StringFind(comment, _ORD_TAKE_PROFIT_ID) + tp_id_len;
    const int   tp_len      = StringFind(comment, _ORD_COMMENT_SEPARATOR, tp_idx) - (tp_idx);
    const int   vol_idx     = StringFind(comment, _ORD_VOLUME_ID) + vol_id_len;
    const int   vol_len     = StringFind(comment, _ORD_COMMENT_SEPARATOR, vol_idx) - (vol_idx);
        const bool  bShort          = (PositionGetInteger(POSITION_TYPE) == POSITION_TYPE_SELL);

    // Évaluate    
    if( (sl_idx == -1)
     || (tp_idx == -1)
     || (vol_idx == -1) )
    { return(false); }

        // Assign values
    sl_ticks = ((long)StringToInteger(StringSubstr(comment, sl_idx, sl_len))) * ((long)(-1 + (2 * ((long)(bShort) & 0x01))));
    tp_ticks = ((long)StringToInteger(StringSubstr(comment, tp_idx, tp_len))) * ((long)(-1 + (2 * ((long)(!bShort) & 0x01))));
    vol = StringToDouble(StringSubstr(comment, vol_idx, vol_len));
    
    // Return
    return(true);
}



After testing with this code, I notice, it is in fact correctly working, but the debugger is showing "scrambled eggs" in the variable-watch list. 

Interesting.

Anyways, thank you for supporting me on my way to find a wanted solution.

Best regards.

Dominik Egert  

As a closing update, here are my three functions as desired in the first place. - Thank you very much for supporting my efforts.

//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const int pos_id)
{
    // Preinit
    data::position_list[pos_id].initial_stop_loss = data::position_list[pos_id].open_price;
    data::position_list[pos_id].initial_take_profit = data::position_list[pos_id].open_price;

    // Return   
    return(pos_initial_geometry(data::position_list[pos_id].ticket,
                                data::position_list[pos_id].initial_stop_loss,
                                data::position_list[pos_id].initial_take_profit,
                                data::position_list[pos_id].initial_volume));
}



//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const ulong ticket, double& abs_sl, double& abs_tp, double& vol)
{
    // Local init
    long sl_ticks = NULL;
    long tp_ticks = NULL;

    // Extract values
    if(!pos_initial_geometry(ticket, sl_ticks, tp_ticks, vol))
    { return(false); }

    // Check input values
    if(ticket > NULL)
    {
        const double open_price = PositionGetDouble(POSITION_PRICE_OPEN);
        abs_sl = (abs_sl * ((abs_sl > NULL) & 0x01)) + (open_price * ((abs_sl == NULL) & 0x01));
        abs_tp = (abs_tp * ((abs_tp > NULL) & 0x01)) + (open_price * ((abs_tp == NULL) & 0x01));
    }
    
    // Calculate absolute values
    const double price_digit_f = MathPow(10.0, SymbolInfoInteger(PositionGetString(POSITION_SYMBOL), SYMBOL_DIGITS));
    abs_sl += (sl_ticks / price_digit_f);
    abs_tp += (tp_ticks / price_digit_f);
    
    // Return
    return(true);
}



//+------------------------------------------------------------------+
//| pos_initial_geometry()                                           |
//+------------------------------------------------------------------+
const bool pos_initial_geometry(const ulong ticket, long& sl_ticks, long& tp_ticks, double& vol)
{
    // Static init
    static const int   sl_id_len   = StringLen(_ORD_STOP_LOSS_ID);
    static const int   tp_id_len   = StringLen(_ORD_TAKE_PROFIT_ID);
    static const int   vol_id_len  = StringLen(_ORD_VOLUME_ID);
    
    // Select position
    if( (ticket > NULL) 
     && !PositionSelectByTicket(ticket) 
     && OnError())
    { return(false); }

    // Get comment string
    const string comment    = PositionGetString(POSITION_COMMENT);

    // Check comment
    const int   sl_idx      = StringFind(comment, _ORD_STOP_LOSS_ID)    + sl_id_len;
    const int   tp_idx      = StringFind(comment, _ORD_TAKE_PROFIT_ID)  + tp_id_len;
    const int   vol_idx     = StringFind(comment, _ORD_VOLUME_ID)       + vol_id_len;

    // Évaluate    
    if( ((sl_idx - sl_id_len) < NULL) 
     || ((tp_idx - tp_id_len) < NULL)
     || ((vol_idx - vol_id_len) < NULL) )
    { return(false); }

    // Get value lengths
    const int   sl_len      = StringFind(comment, _ORD_COMMENT_SEPARATOR, sl_idx)   - (sl_idx);
    const int   tp_len      = StringFind(comment, _ORD_COMMENT_SEPARATOR, tp_idx)   - (tp_idx);
    const int   vol_len     = StringFind(comment, _ORD_COMMENT_SEPARATOR, vol_idx)  - (vol_idx);
    const bool  bShort      = (PositionGetInteger(POSITION_TYPE) == POSITION_TYPE_SELL);

    // Extract values
    sl_ticks    = ((long)StringToInteger(StringSubstr(comment, sl_idx, sl_len))) * ((long)(-1 + (2 * (( bShort) & 0x01))));
    tp_ticks    = ((long)StringToInteger(StringSubstr(comment, tp_idx, tp_len))) * ((long)(-1 + (2 * ((!bShort) & 0x01))));
    vol         = StringToDouble(StringSubstr(comment, vol_idx, vol_len));
    
    // Return
    return(true);
}
Documentation on MQL5: Constants, Enumerations and Structures / Named Constants / Predefined Macro Substitutions
Documentation on MQL5: Constants, Enumerations and Structures / Named Constants / Predefined Macro Substitutions
  • www.mql5.com
Predefined Macro Substitutions - Named Constants - Constants, Enumerations and Structures - MQL5 Reference - Reference on algorithmic/automated trading language for MetaTrader 5
lippmaje  
You're welcome. If you want to optimize speed you may do it of course, but keep in mind that most of the cycles are spent waiting for incoming ticks.
Dominik Egert  
lippmaje:
You're welcome. If you want to optimize speed you may do it of course, but keep in mind that most of the cycles are spent waiting for incoming ticks.

I have digged into the issue, i was having and i found the following to be true.

long l_var = -1;
int  i_var = -1;

Here both values assigned to the variables are interpretedcorrectly, respecting the amount of bits the target has. 

But when using this code:

bool bShort = false;
long sl_ticks = 100 * ((-1 + (2 * ((bShort) & 0x01))));

the values inside the brackets get interpreted as 32 bit values, therefore they mess up the signed-bit given.

So sl_ticks ends up having the value: 4294967196 or 0xFFFFFFFF9C 

The Hex value shown is in fact the signed representation of the decimal value 100.


According to 

C99 standard, clause 6.4.4:5. For a decimal constant, the list is  int ,  long int ,  long long int

this behaviour is correct.


So I tried to define the given literal -1 as follows:

long sl_ticks = 100 * ((-1L + (2 * ((bShort) & 0x01))));

In hte hope, the compiler would interpret the literal -1L as a long value represented by 64 bit.

But it did not. - It ignored my wish.


So the solution to the issue was to use a casting-operator as shown here:

long sl_ticks = 100 * (((long)-1 + (2 * ((bShort) & 0x01))));

This has fixed the issue at the root, at least as close as possible.

Nether the less, thank you for your support.

Reason: