Compiler code generating error
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.
fix:
or just
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.
- www.mql5.com
fix:
or just
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/
- www.mql5.com
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.
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.
- www.mql5.com
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.
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.
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); }
- www.mql5.com
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.
- Free trading apps
- Over 8,000 signals for copying
- Economic news for exploring financial markets
You agree to website policy and terms of use
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:
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:
These functions are my current actual implementation.