Skip to content

C++: Add a sample class in PrintAST.ql#2188

Merged
dbartol merged 2 commits intogithub:masterfrom
jbj:printast-override
Nov 1, 2019
Merged

C++: Add a sample class in PrintAST.ql#2188
dbartol merged 2 commits intogithub:masterfrom
jbj:printast-override

Conversation

@jbj
Copy link
Contributor

@jbj jbj commented Oct 24, 2019

I've found myself typing out this class whenever I want to print the AST of one function. I hope it will be useful to others too.

Or is there a better way?

I've found myself typing out this class whenever I want to print the AST
of one function. I hope it will be useful to others too.
@jbj jbj added the C++ label Oct 24, 2019
@jbj jbj requested a review from dave-bartolomeo October 24, 2019 12:48
@jbj jbj requested a review from a team as a code owner October 24, 2019 12:48
zlaski-semmle
zlaski-semmle previously approved these changes Oct 24, 2019
* printed.
*/
class Cfg extends PrintASTConfiguration {
/** Holds if the AST for `func` should be printed. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add Tweak this predicate as needed, perhaps in all caps. Otherwise LGTM.

geoffw0
geoffw0 previously approved these changes Oct 24, 2019
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jbj jbj dismissed stale reviews from geoffw0 and zlaski-semmle via 8f58e7e October 24, 2019 15:34
@jbj
Copy link
Contributor Author

jbj commented Oct 24, 2019

I'm happy to use all caps here, especially since I'm not the one who suggested it :). Done.

@dbartol
Copy link

dbartol commented Oct 24, 2019

What's the typical usage? Copy PrintAST.ql locally and tweak the condition?

@jbj
Copy link
Contributor Author

jbj commented Oct 25, 2019

I always tweak PrintAST.ql directly (and try to be careful not to commit my changes). For more long-lived uses, it would make sense to copy it.

Copy link

@dbartol dbartol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dbartol dbartol merged commit ea23c2d into github:master Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants