const arguments to matchIRExpr()

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

const arguments to matchIRExpr()

iraisr
Hi developers,

Are there any objections to the changes for this bug:
https://bugs.kde.org/show_bug.cgi?id=373938

The gist of my changes is really just:
- Bool matchIRExpr ( MatchInfo* mi, IRExpr* p, IRExpr* e );
+ Bool matchIRExpr ( MatchInfo* mi, const IRExpr* p, const IRExpr* e );

However this resulted in some more 'const' modifiers thrown
into various VEX/priv/host_*_isel.c files. Nothing more.

I.

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: const arguments to matchIRExpr()

John Reiser
On 12/21/2016 12:57 AM, Ivo Raisr wrote:

> Are there any objections to the changes for this bug:
> https://bugs.kde.org/show_bug.cgi?id=373938
>
> The gist of my changes is really just:
> - Bool matchIRExpr ( MatchInfo* mi, IRExpr* p, IRExpr* e );
> + Bool matchIRExpr ( MatchInfo* mi, const IRExpr* p, const IRExpr* e );

I applaud the use of 'const' as often as possible.
It increases understanding and reduces debugging and maintenance costs
because it reduces the places where values can change.
It also tells the compiler where aggressive optimization is possible.

I believe that 'const' should appear as far to the right as possible:

        IRExpr const *p

This makes declarations easier to read.  Declarations are read
from the inside to the outside; this means right-to-left
(except for function names and order of parameters.)
In the example, what is 'p'?

        spoken or read: p is a pointer to a const IRExpr
        transliterated: p      *            const IRExpr
        proper syntax:  IRExpr const *p

This placement of 'const' is especially natural in C++,
where the spoken "by const-ref" becomes the syntax "const&".




------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: const arguments to matchIRExpr()

Paul Floyd


----- Original Message -----

> On 12/21/2016 12:57 AM, Ivo Raisr wrote:
>
> > Are there any objections to the changes for this bug:
> > https://bugs.kde.org/show_bug.cgi?id=373938
> >
> > The gist of my changes is really just:
> > - Bool matchIRExpr ( MatchInfo* mi, IRExpr* p, IRExpr* e );
> > + Bool matchIRExpr ( MatchInfo* mi, const IRExpr* p, const IRExpr*
> > e );
>
> I applaud the use of 'const' as often as possible.
> It increases understanding and reduces debugging and maintenance
> costs
> because it reduces the places where values can change.
> It also tells the compiler where aggressive optimization is possible.


Unfortunately, mainly due to pointers/references and aliasing, this isn't as often as one might hope.

For instance see

http://www.gotw.ca/gotw/081.htm

> I believe that 'const' should appear as far to the right as possible:
>
> IRExpr const *p
>
> This makes declarations easier to read.  Declarations are read
> from the inside to the outside; this means right-to-left
> (except for function names and order of parameters.)
> In the example, what is 'p'?

Another link for reading declarations : http://cdecl.org
It's somewhat tongue in cheek, and it doesn't support much if any C++.
 
> spoken or read: p is a pointer to a const IRExpr
> transliterated: p      *            const IRExpr
> proper syntax:  IRExpr const *p
>
> This placement of 'const' is especially natural in C++,
> where the spoken "by const-ref" becomes the syntax "const&".

Personally I don't mind either, as long as it's consistent.

A+
Paul

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: const arguments to matchIRExpr()

iraisr
In reply to this post by John Reiser


2016-12-21 17:08 GMT+01:00 John Reiser <[hidden email]>:
On 12/21/2016 12:57 AM, Ivo Raisr wrote:

> Are there any objections to the changes for this bug:
> https://bugs.kde.org/show_bug.cgi?id=373938
>
> The gist of my changes is really just:
> - Bool matchIRExpr ( MatchInfo* mi, IRExpr* p, IRExpr* e );
> + Bool matchIRExpr ( MatchInfo* mi, const IRExpr* p, const IRExpr* e );

I applaud the use of 'const' as often as possible.
It increases understanding and reduces debugging and maintenance costs
because it reduces the places where values can change.
It also tells the compiler where aggressive optimization is possible.

I believe that 'const' should appear as far to the right as possible:

        IRExpr const *p

This makes declarations easier to read.  Declarations are read
from the inside to the outside; this means right-to-left
(except for function names and order of parameters.)
In the example, what is 'p'?

        spoken or read: p is a pointer to a const IRExpr
        transliterated: p      *            const IRExpr
        proper syntax:  IRExpr const *p

This placement of 'const' is especially natural in C++,
where the spoken "by const-ref" becomes the syntax "const&".

Thank you for detailed explanation of your arguments.
I think consistency with the rest of VEX code is also important.
I did a little research and nowhere in VEX we can see 'T const *';
it is always 'const T *'.

I.


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: const arguments to matchIRExpr()

iraisr
In reply to this post by Paul Floyd


2016-12-21 21:29 GMT+01:00 <[hidden email]>:

> It also tells the compiler where aggressive optimization is possible.


Unfortunately, mainly due to pointers/references and aliasing, this isn't as often as one might hope.

For instance see

http://www.gotw.ca/gotw/081.htm

An interesting reading, indeed.

Me being not a great C++ coder, would some please mind explaining the meaning of "const" here for this method:
char operator[]( size_t ) const;

I.


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: const arguments to matchIRExpr()

Paul Floyd

On 23 Dec 2016, at 04:04, Ivo Raisr wrote:



2016-12-21 21:29 GMT+01:00 <[hidden email]>:

> It also tells the compiler where aggressive optimization is possible.


Unfortunately, mainly due to pointers/references and aliasing, this isn't as often as one might hope.

For instance see

http://www.gotw.ca/gotw/081.htm

An interesting reading, indeed.

Me being not a great C++ coder, would some please mind explaining the meaning of "const" here for this method:
char operator[]( size_t ) const;

Hi Ivo

If you have a class C with method f() like this

class C
{
public:
    void f();
};

then in C++ f has an 'implicit this', which means that the actual code generated will be like

void C::f(C* this).

Since the 'this' pointer is implicit, there's no way to make it const in the formal parameter list, i.e., you can't write

void C:f(C const* this).

So in order to make the 'this' pointer const, the language allows putting const after the member declaration,, i.e.,

class C
{
public:
    void f() const;
};

A+
Paul


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Valgrind-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
Loading...