[llvm-commits] [PATCH Proposal] Clean up Support/Process.h to modern llvm style.

[Sean Silva]

You should write up an LLVM doxygen quickstart tutorial to get people
up to speed with the expected doxygen standards, since you seem to
have a very clear picture of what is acceptable. Given the content of
your post, these conventions have grown beyond just preferences and
are now blocking patches and being considered part of "modern LLVM
style". I think the community will appreciate getting them out in the
open and written down.

More importantly, mechanical source code issues like this should not
waste reviewer time. You should be able to say "please review
llvm.org/docs/DoxygenQuickstart.html" and continue on to more
productive comments.

Another possibility is putting the rules in the LLVM Coding Standards,
but I think a "quickstart tutorial" format is a lot better because it
is much more inviting to readers and allows the exposition to more
naturally demonstrate the rules by example _in context_ (rather than
simply a bland "do this, don't do this, etc."); you can also easily
sprinkle reStructuredText admonitions throughout the tutorial to call
out specific practices as truly being "rules" (let me know if you need
any help with the reST). Of course, a link from the Coding Standards
to the doxygen quickstart should probably also be supplied.

Of course, if you really want to be an overachiever, you can do both a
quickstart tutorial and put an explicit do/don't list in the coding
standards ;)

--Sean Silva

On Fri, Aug 10, 2012 at 5:57 PM, Chandler Carruth <chandlerc at google.com> wrote:
> Your subject line is a bit misleading, it's not yet in modern LLVM style...
> ;] Still, an improvement.
>
> >From 049e77c972ab35097b81c78db478546c8e2bc928 Mon Sep 17 00:00:00 2001
> From: Michael Spencer <bigcheesegs at gmail.com>
> Date: Tue, 7 Aug 2012 12:50:19 -0700
> Subject: [PATCH] [Support] Clean up the Process interface to modern llvm
>  style.
>
> ---
>  include/llvm/Support/Process.h |  110
> ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 110 insertions(+)
>
> diff --git a/include/llvm/Support/Process.h b/include/llvm/Support/Process.h
> index 088897c..c290d22 100644
> --- a/include/llvm/Support/Process.h
> +++ b/include/llvm/Support/Process.h
> @@ -18,6 +18,116 @@
>
>  namespace llvm {
>  namespace sys {
> +/// Get the virtual memory page size
>
> Use \brief.
>
> +///
> +/// \returns The number of bytes in a virtual memory page.
> +unsigned getPageSize();
> +
> +/// Get process memory usage.
> +///
> +/// This function will return the total amount of memory allocated by the
> +/// process. This only counts the memory allocated via the malloc, calloc
> and
> +/// realloc functions and includes any "free" holes in the allocated space.
> +size_t getMallocUsage();
> +
> +/// This function will set \p user_time to the amount of CPU time spent in
> user
> +/// (non-kernel) mode and \p sys_time to the amount of CPU time spent in
> system
> +/// (kernel) mode.  If the operating system does not support collection of
> these
> +/// metrics, a zero TimeValue will be for both values.
> +void getTimeUsage(
> +  TimeValue &Elapsed,
> +    ///< Returns the TimeValue::now() giving current time
>
> Please no, this makes the actual signatures nearly impossible to read.
>
> Use the much more widespread '\param' tag in the body of the doxygen comment
> above.
>
> +  TimeValue &UserTime,
> +    ///< Returns the current amount of user time for the process
> +  TimeValue &SysTime
> +    ///< Returns the current amount of system time for the process
> +  );
>
> And never put this on the next line. It makes it far too easy to omit or add
> extraneous ','s after parameters.
>
> Also, by splitting every bit of this declaration across so many lines you
> make the diagnostic quality worse because we can't snippet anything
> effectively.
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>