On Sat, 2021-07-24 at 15:22 +0530, Ankur Saini wrote:
> The patch generalises how analyzer's call_string tracks function calls,
> including the calls that doesn’t have an underlying call graph edge,
> making it possible to work with function calls which are not discovered
> by GCC's middle end.
>
> Successfully bootstrapped and completed regress tests on x86_64-linux-
> gnu.
Great!
[...snip...]
I have some very minor nitpicks for you to fix before you commit this.
Sorry if this seems overly pedantic, but it's good to get into the
habit of following the project's coding conventions...
Sentences in comments should be captitalized, so:
> +/* return the opinter to callee of the topmost call in the stack,
~~~~~~~~~~~~~~~~~~
"Return the pointer"
> + or NULL if stack is empty */
Our convention is a trailing period/full-stop then two spaces, so:
or NULL if stack is empty. */
> +const supernode *
> +call_string::get_callee_node () const
> +{
> + if(m_elements.is_empty ())
> + return NULL;
> + return m_elements[m_elements.length () - 1].m_callee;
> +}
> +
> +/* return the pointer to caller of the topmost call in the stack,
> + or NULL if stack is empty */
Similarly, initial capital for "return" at the start of sentence, full-stop
at the end, then two spaces before terminating the comment.
> +const supernode *
> +call_string::get_caller_node () const
> +{
> + if(m_elements.is_empty ())
> + return NULL;
> + return m_elements[m_elements.length () - 1].m_caller;
> +}
> +
[...snip...]
> /* A string of return_superedge pointers, representing a call stack
> at a program point.
>
> This is used to ensure that we generate interprocedurally valid paths
> - i.e. that we return to the same callsite that called us.
> + i.e. that we return to the same callsite that called us.
The patch adds a trailing whitespace to the above line. Please try to
avoid such unnecessary changes as they create noise in "git blame" and
other logs.
> - The class actually stores the return edges, rather than the call edges,
> - since that's what we need to compare against. */
> + The class stores returing calls ( which may be represented by a
Typo: "returing" -> "returning"
> + returning superedge ). We do so because this is what we need to compare
> + against. */
>
> class call_string
> {
> public:
> - call_string () : m_return_edges () {}
> + /* A struct representing an element in the call_string
> +
> + each element represnts a path from m_callee to m_caller which represents
~~~~ ~~~~~~~~~
Each represents
> + returning from the called function */
~~~~~~~~~~~
function. */
[...snip...]
With those minor nitpicks, the patch is ready to commit, so please go
ahead and push it to trunk.
Thanks
Dave
