Is it safe to cast-away constness to use Clang static analysis?

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev
Hello,

I'm writing a code refactoring tool that needs some static analysis capabilities.
I've noticed that AST Matchers return const AST nodes, but static analysis sometimes wants to consume non-const pointers.

For example to build call graph I need to pass non-const pointer:
 CallGraph::addToCallGraph(Decl *D)

So does static analysis modifies AST? Or it's just a bug in method signature?

Thanks,
Roman

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev

Hi,

you can try `const_cast` as first try to do it.

Jonas


Am 03.10.2017 um 21:31 schrieb Roman Popov via cfe-dev:
Hello,

I'm writing a code refactoring tool that needs some static analysis capabilities.
I've noticed that AST Matchers return const AST nodes, but static analysis sometimes wants to consume non-const pointers.

For example to build call graph I need to pass non-const pointer:
 CallGraph::addToCallGraph(Decl *D)

So does static analysis modifies AST? Or it's just a bug in method signature?

Thanks,
Roman


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev
Yes, this is what I have to do.

Actually RecursiveASTVisitor does not enforce constness, so probably it is AST matchers that are not consistent with rest of Clang API. 

-Roman

2017-10-03 12:49 GMT-07:00 Jonas Toth <[hidden email]>:

Hi,

you can try `const_cast` as first try to do it.

Jonas


Am 03.10.2017 um 21:31 schrieb Roman Popov via cfe-dev:
Hello,

I'm writing a code refactoring tool that needs some static analysis capabilities.
I've noticed that AST Matchers return const AST nodes, but static analysis sometimes wants to consume non-const pointers.

For example to build call graph I need to pass non-const pointer:
 CallGraph::addToCallGraph(Decl *D)

So does static analysis modifies AST? Or it's just a bug in method signature?

Thanks,
Roman


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev
On Oct 3, 2017, at 4:51 PM, Roman Popov via cfe-dev <[hidden email]> wrote:
Yes, this is what I have to do.

Actually RecursiveASTVisitor does not enforce constness, so probably it is AST matchers that are not consistent with rest of Clang API. 

Clang is unfortunately just inconsistent about it.

The AST is largely immutable by design, and the best solution would probably be to adopt what LLVM did to Type and just mass-refactor code to stop passing around const pointers at all.

John.


-Roman

2017-10-03 12:49 GMT-07:00 Jonas Toth <[hidden email]>:

Hi,

you can try `const_cast` as first try to do it.

Jonas


Am 03.10.2017 um 21:31 schrieb Roman Popov via cfe-dev:
Hello,

I'm writing a code refactoring tool that needs some static analysis capabilities.
I've noticed that AST Matchers return const AST nodes, but static analysis sometimes wants to consume non-const pointers.

For example to build call graph I need to pass non-const pointer:
 CallGraph::addToCallGraph(Decl *D)

So does static analysis modifies AST? Or it's just a bug in method signature?

Thanks,
Roman


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev
On Wed, Oct 4, 2017 at 2:20 AM, John McCall via cfe-dev
<[hidden email]> wrote:
>
> The AST is largely immutable by design, and the best solution would probably
> be to adopt what LLVM did to Type and just mass-refactor code to stop
> passing around const pointers at all.

If the AST is meant to be immutable, why not pass const
{pointers,references} around?

Csaba
--
GCS a+ e++ d- C++ ULS$ L+$ !E- W++ P+++$ w++$ tv+ b++ DI D++ 5++
The Tao of math: The numbers you can count are not the real numbers.
Life is complex, with real and imaginary parts.
"Ok, it boots. Which means it must be bug-free and perfect. " -- Linus Torvalds
"People disagree with me. I just ignore them." -- Linus Torvalds
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev

> On Oct 4, 2017, at 3:30 AM, Csaba Raduly via cfe-dev <[hidden email]> wrote:
>
> On Wed, Oct 4, 2017 at 2:20 AM, John McCall via cfe-dev
> <[hidden email]> wrote:
>>
>> The AST is largely immutable by design, and the best solution would probably
>> be to adopt what LLVM did to Type and just mass-refactor code to stop
>> passing around const pointers at all.
>
> If the AST is meant to be immutable, why not pass const
> {pointers,references} around?

If the objects are actually immutable, const is just noise repeated everywhere, because a non-const object is basically a useless type.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev
In conclusion: 
Should AST matchers should return non-const pointers to comply with Clang coding standard for immutable objects?

2017-10-04 11:14 GMT-07:00 John McCall via cfe-dev <[hidden email]>:

> On Oct 4, 2017, at 3:30 AM, Csaba Raduly via cfe-dev <[hidden email]> wrote:
>
> On Wed, Oct 4, 2017 at 2:20 AM, John McCall via cfe-dev
> <[hidden email]> wrote:
>>
>> The AST is largely immutable by design, and the best solution would probably
>> be to adopt what LLVM did to Type and just mass-refactor code to stop
>> passing around const pointers at all.
>
> If the AST is meant to be immutable, why not pass const
> {pointers,references} around?

If the objects are actually immutable, const is just noise repeated everywhere, because a non-const object is basically a useless type.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev
On Oct 4, 2017, at 2:21 PM, Roman Popov <[hidden email]> wrote:
In conclusion: 
Should AST matchers should return non-const pointers to comply with Clang coding standard for immutable objects?

I think it is more likely than not that the Clang ASTs will gradually standardize on taking non-const pointers.  Given that AST matchers tend to create out-of-tree dependencies, I think it would be better for them to go ahead and work primarily with non-const pointers.

If you want to do some of the work to make the ASTs traffic in non-const pointers, that would be good, too, although we should first get consensus that that's the direction we want to go in.

John.


2017-10-04 11:14 GMT-07:00 John McCall via cfe-dev <[hidden email]>:

> On Oct 4, 2017, at 3:30 AM, Csaba Raduly via cfe-dev <[hidden email]> wrote:
>
> On Wed, Oct 4, 2017 at 2:20 AM, John McCall via cfe-dev
> <[hidden email]> wrote:
>>
>> The AST is largely immutable by design, and the best solution would probably
>> be to adopt what LLVM did to Type and just mass-refactor code to stop
>> passing around const pointers at all.
>
> If the AST is meant to be immutable, why not pass const
> {pointers,references} around?

If the objects are actually immutable, const is just noise repeated everywhere, because a non-const object is basically a useless type.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev
Well, after a couple of weeks of playing with developing toy Clang-based tool I came to conclusion that it is better to create pointers to const AST nodes.
For two reasons:
1) ast Matchers are widely used in practice. They are often more convenient than writing custom ast visitors for every small subtree search.
And I don't want to do const_cast each time you use ast Matcher.
2)  Most standard C++ classes are mutable, so it's hard to explain non-Clang developer that you omitting const here because object is immutable by design

So it's easier to const than not to const...


2017-10-04 11:40 GMT-07:00 John McCall <[hidden email]>:
On Oct 4, 2017, at 2:21 PM, Roman Popov <[hidden email]> wrote:
In conclusion: 
Should AST matchers should return non-const pointers to comply with Clang coding standard for immutable objects?

I think it is more likely than not that the Clang ASTs will gradually standardize on taking non-const pointers.  Given that AST matchers tend to create out-of-tree dependencies, I think it would be better for them to go ahead and work primarily with non-const pointers.

If you want to do some of the work to make the ASTs traffic in non-const pointers, that would be good, too, although we should first get consensus that that's the direction we want to go in.

John.


2017-10-04 11:14 GMT-07:00 John McCall via cfe-dev <[hidden email]>:

> On Oct 4, 2017, at 3:30 AM, Csaba Raduly via cfe-dev <[hidden email]> wrote:
>
> On Wed, Oct 4, 2017 at 2:20 AM, John McCall via cfe-dev
> <[hidden email]> wrote:
>>
>> The AST is largely immutable by design, and the best solution would probably
>> be to adopt what LLVM did to Type and just mass-refactor code to stop
>> passing around const pointers at all.
>
> If the AST is meant to be immutable, why not pass const
> {pointers,references} around?

If the objects are actually immutable, const is just noise repeated everywhere, because a non-const object is basically a useless type.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev




_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Is it safe to cast-away constness to use Clang static analysis?

Eric Fiselier via cfe-dev
On Oct 13, 2017, at 9:13 PM, Roman Popov <[hidden email]> wrote:
Well, after a couple of weeks of playing with developing toy Clang-based tool I came to conclusion that it is better to create pointers to const AST nodes.
For two reasons:
1) ast Matchers are widely used in practice. They are often more convenient than writing custom ast visitors for every small subtree search.
And I don't want to do const_cast each time you use ast Matcher.
2)  Most standard C++ classes are mutable, so it's hard to explain non-Clang developer that you omitting const here because object is immutable by design

So it's easier to const than not to const...

You could make the matchers take const pointers by default.  But ultimately it's your API; do what you want.

John.



2017-10-04 11:40 GMT-07:00 John McCall <[hidden email]>:
On Oct 4, 2017, at 2:21 PM, Roman Popov <[hidden email]> wrote:
In conclusion: 
Should AST matchers should return non-const pointers to comply with Clang coding standard for immutable objects?

I think it is more likely than not that the Clang ASTs will gradually standardize on taking non-const pointers.  Given that AST matchers tend to create out-of-tree dependencies, I think it would be better for them to go ahead and work primarily with non-const pointers.

If you want to do some of the work to make the ASTs traffic in non-const pointers, that would be good, too, although we should first get consensus that that's the direction we want to go in.

John.


2017-10-04 11:14 GMT-07:00 John McCall via cfe-dev <[hidden email]>:

> On Oct 4, 2017, at 3:30 AM, Csaba Raduly via cfe-dev <[hidden email]> wrote:
>
> On Wed, Oct 4, 2017 at 2:20 AM, John McCall via cfe-dev
> <[hidden email]> wrote:
>>
>> The AST is largely immutable by design, and the best solution would probably
>> be to adopt what LLVM did to Type and just mass-refactor code to stop
>> passing around const pointers at all.
>
> If the AST is meant to be immutable, why not pass const
> {pointers,references} around?

If the objects are actually immutable, const is just noise repeated everywhere, because a non-const object is basically a useless type.

John.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev





_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev