fix doxygen brief in subzero header files

fix doxygen brief in subzero header files

There is a consistent mistake in most of the file level subzero doxygen
comments which stems from a problem with brief.

Basically, there are two types of documentation in Doxygen, inline and
structural documentation. File level documentation is always structural
because inline documentation must either come before or after the entity
and for a file that would be impossible.

When you have structure documentation, the brief and detailed is
handled differently and you have to explicity put the \brief qualifier in.

We were only doing this for a few files so when you looked the doxygen
for the source directory, the description was blank for almost all of them.
If you build docs with this patch and look at the "files" and src
directory, you will see all the brief descriptions now.

In addition I deleted the text "This file ..." from the
beginning of all the file desriptions because it's redundant when you look at the doxygen output and adds
no information but takes up space in the descripton.
BUG=
R=stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=92a6e5b08ec68e7076d637ebc680da2fcc346a00

[rkotlerimgtec, 2015-12-01 01:25:40]

Description was changed from

==========
fix doxygen brief in subzero header files

There is a consistent mistake in most of the file level subzero doxygen
comments which stems from a problem with brief.

Basically, there are two types of documentation in Doxygen, inline and
structural documentation. File level documentation is always structural
because inline documentation must either come before or after the entity
and for a file that would be impossible.

When you have structure documentation, the brief and detailed is
handled differently and you have to explicity put the \brief qualifier in.

We were only doing this for a few files so when you looked the doxygen
for the source directory, the description was blank for almost all of them.
If you build docs with this patch and look at the "files" and src
directory, you will see all the brief descriptions now.

BUG=
==========

to

==========
fix doxygen brief in subzero header files

There is a consistent mistake in most of the file level subzero doxygen
comments which stems from a problem with brief.

Basically, there are two types of documentation in Doxygen, inline and
structural documentation. File level documentation is always structural
because inline documentation must either come before or after the entity
and for a file that would be impossible.

When you have structure documentation, the brief and detailed is
handled differently and you have to explicity put the \brief qualifier in.

We were only doing this for a few files so when you looked the doxygen
for the source directory, the description was blank for almost all of them.
If you build docs with this patch and look at the "files" and src
directory, you will see all the brief descriptions now.

BUG=
==========

[rkotlerimgtec, 2015-12-01 01:25:40]

rkotlerimgtec@gmail.com changed reviewers:
+ jpp@chromium.org, kschimpf@chromium.org, stichnot@chromium.org

[rkotlerimgtec, 2015-12-01 01:29:51]

Description was changed from

==========
fix doxygen brief in subzero header files

There is a consistent mistake in most of the file level subzero doxygen
comments which stems from a problem with brief.

Basically, there are two types of documentation in Doxygen, inline and
structural documentation. File level documentation is always structural
because inline documentation must either come before or after the entity
and for a file that would be impossible.

When you have structure documentation, the brief and detailed is
handled differently and you have to explicity put the \brief qualifier in.

We were only doing this for a few files so when you looked the doxygen
for the source directory, the description was blank for almost all of them.
If you build docs with this patch and look at the "files" and src
directory, you will see all the brief descriptions now.

BUG=
==========

to

==========
fix doxygen brief in subzero header files

There is a consistent mistake in most of the file level subzero doxygen
comments which stems from a problem with brief.

Basically, there are two types of documentation in Doxygen, inline and
structural documentation. File level documentation is always structural
because inline documentation must either come before or after the entity
and for a file that would be impossible.

When you have structure documentation, the brief and detailed is
handled differently and you have to explicity put the \brief qualifier in.

We were only doing this for a few files so when you looked the doxygen
for the source directory, the description was blank for almost all of them.
If you build docs with this patch and look at the "files" and src
directory, you will see all the brief descriptions now.

In addition I deleted the text "This file ..." from the 
beginning of all the file desriptions because it's redundant when you look at
the doxygen output and adds
no information but takes up space in the descripton.
BUG=
==========

[Rich.Fuhler_imgtec.com, 2015-12-01 01:45:13]

Isn't this type of work NFC rather than BUG? Just curious...



Sent from my Verizon Wireless 4G LTE smartphone


-------- Original message --------
From: rkotlerimgtec@gmail.com
Date: 11/30/2015 17:27 (GMT-08:00)
To: stichnot@chromium.org, jpp@chromium.org, kschimpf@chromium.org
Cc: native-client-reviews@googlegroups.com, Rich Fuhler
<Rich.Fuhler@imgtec.com>
Subject: fix doxygen brief in subzero header files (issue 1488913002 by
rkotlerimgtec@gmail.com)

Reviewers: stichnot, John, kschimpf
CL: https://codereview.chromium.org/1488913002/

Description:
fix doxygen brief in subzero header files

There is a consistent mistake in most of the file level subzero doxygen
comments which stems from a problem with brief.

Basically, there are two types of documentation in Doxygen, inline and
structural documentation. File level documentation is always structural
because inline documentation must either come before or after the entity
and for a file that would be impossible.

When you have structure documentation, the brief and detailed is
handled differently and you have to explicity put the \brief qualifier in.

We were only doing this for a few files so when you looked the doxygen
for the source directory, the description was blank for almost all of them.
If you build docs with this patch and look at the "files" and src
directory, you will see all the brief descriptions now.

BUG=

Base URL:
https://chromium.googlesource.com/native_client/pnacl-subzero.git@master

Affected files (+201, -168 lines):
   M src/IceAPFloat.h
   M src/IceAPInt.h
   M src/IceAssembler.h
   M src/IceAssembler.cpp
   M src/IceAssemblerARM32.h
   M src/IceAssemblerARM32.cpp
   M src/IceAssemblerMIPS32.h
   M src/IceAssemblerX8632.h
   M src/IceAssemblerX8664.h
   M src/IceAssemblerX86Base.h
   M src/IceAssemblerX86BaseImpl.h
   M src/IceBrowserCompileServer.h
   M src/IceBrowserCompileServer.cpp
   M src/IceBuildDefs.h
   M src/IceCfg.h
   M src/IceCfg.cpp
   M src/IceCfgNode.h
   M src/IceCfgNode.cpp
   M src/IceClFlags.h
   M src/IceClFlags.cpp
   M src/IceClFlagsExtra.h
   M src/IceCompileServer.h
   M src/IceCompileServer.cpp
   M src/IceCompiler.h
   M src/IceCompiler.cpp
   M src/IceConditionCodesARM32.h
   M src/IceConditionCodesX8632.h
   M src/IceConditionCodesX8664.h
   M src/IceConverter.h
   M src/IceConverter.cpp
   M src/IceDefs.h
   M src/IceELFObjectWriter.h
   M src/IceELFObjectWriter.cpp
   M src/IceELFSection.h
   M src/IceELFSection.cpp
   M src/IceELFStreamer.h
   M src/IceFixups.h
   M src/IceFixups.cpp
   M src/IceGlobalContext.h
   M src/IceGlobalContext.cpp
   M src/IceGlobalInits.h
   M src/IceGlobalInits.cpp
   M src/IceInst.h
   M src/IceInst.cpp
   M src/IceInstARM32.h
   M src/IceInstARM32.cpp
   M src/IceInstMIPS32.h
   M src/IceInstMIPS32.cpp
   M src/IceInstVarIter.h
   M src/IceInstX8632.h
   M src/IceInstX8632.cpp
   M src/IceInstX8664.h
   M src/IceInstX8664.cpp
   M src/IceInstX86Base.h
   M src/IceInstX86BaseImpl.h
   M src/IceIntrinsics.h
   M src/IceIntrinsics.cpp
   M src/IceLiveness.h
   M src/IceLiveness.cpp
   M src/IceLoopAnalyzer.cpp
   M src/IceOperand.h
   M src/IceOperand.cpp
   M src/IcePhiLoweringImpl.h
   M src/IceRNG.h
   M src/IceRNG.cpp
   M src/IceRegAlloc.h
   M src/IceRegAlloc.cpp
   M src/IceRegList.h
   M src/IceRegistersARM32.h
   M src/IceRegistersMIPS32.h
   M src/IceRegistersX8632.h
   M src/IceRegistersX8664.h
   M src/IceSwitchLowering.h
   M src/IceSwitchLowering.cpp
   M src/IceTLS.h
   M src/IceTargetLowering.h
   M src/IceTargetLowering.cpp
   M src/IceTargetLoweringARM32.h
   M src/IceTargetLoweringARM32.cpp
   M src/IceTargetLoweringMIPS32.h
   M src/IceTargetLoweringMIPS32.cpp
   M src/IceTargetLoweringX8632.h
   M src/IceTargetLoweringX8632.cpp
   M src/IceTargetLoweringX8632Traits.h
   M src/IceTargetLoweringX8664.h
   M src/IceTargetLoweringX8664.cpp
   M src/IceTargetLoweringX8664Traits.h
   M src/IceTargetLoweringX86Base.h
   M src/IceTargetLoweringX86BaseImpl.h
   M src/IceTargetLoweringX86RegClass.h
   M src/IceThreading.h
   M src/IceThreading.cpp
   M src/IceTimerTree.h
   M src/IceTimerTree.cpp
   M src/IceTranslator.h
   M src/IceTranslator.cpp
   M src/IceTypeConverter.h
   M src/IceTypeConverter.cpp
   M src/IceTypes.h
   M src/IceTypes.cpp
   M src/IceUtils.h
   M src/PNaClTranslator.h
   M src/PNaClTranslator.cpp
   M src/main.cpp


-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/d/optout.

[native-client-reviews_googlegroups.com, 2015-12-01 14:57:37]

On Mon, Nov 30, 2015 at 5:45 PM, Rich Fuhler <Rich.Fuhler@imgtec.com> wrote:

> Isn't this type of work NFC rather than BUG? Just curious...
>

For better or worse, NaCl CLs usually use "BUG= none" if there's no
suitable issue to link to.  Note the space after the equals character, to
superstitiously ensure defeat over the issue auto-linking mechanism. :)

Jim

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/d/optout.

[Jim Stichnoth, 2015-12-01 18:41:17]

Thanks!

https://codereview.chromium.org/1488913002/diff/1/src/IceAPFloat.h
File src/IceAPFloat.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceAPFloat.h#newcode11
src/IceAPFloat.h:11: /// \brief Implements a class to represent float and double
Reflow to 80-col

https://codereview.chromium.org/1488913002/diff/1/src/IceAPInt.h
File src/IceAPInt.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceAPInt.h#newcode11
src/IceAPInt.h:11: /// \brief Implements a class to represent 64 bit integer
constant
Reflow to 80-col

https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerARM32.h#new...
src/IceAssemblerARM32.h:19: /// \brief Implements the Assembler class for ARM32.
The .cpp file also says "implements".  In some other cases, the .h file says
"declares" while the .cpp file says "implements".

Do we / should we have a rule for "implements" vs "declares"?

https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerX86Base.h
File src/IceAssemblerX86Base.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerX86Base.h#n...
src/IceAssemblerX86Base.h:19: /// \brief Defines the AssemblerX86 template class
for x86, the base of all
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.cpp
File src/IceCfg.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.cpp#newcode11
src/IceCfg.cpp:11: /// \brief Implements the Cfg class, including constant pool
management.
I don't think Cfg has anything to do with constant pool management.  I wonder if
that was a copy&paste mistake.  Just remove that clause?

https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.h
File src/IceCfg.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.h#newcode11
src/IceCfg.h:11: /// \brief Declares the Cfg class, which represents the control
flow graph
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.cpp
File src/IceCfgNode.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.cpp#newcode11
src/IceCfgNode.cpp:11: /// \brief Implements the CfgNode class, including the
complexities
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.h
File src/IceCfgNode.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.h#newcode11
src/IceCfgNode.h:11: /// \brief Declares the CfgNode class, which represents a
single basic block
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceClFlagsExtra.h
File src/IceClFlagsExtra.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceClFlagsExtra.h#newco...
src/IceClFlagsExtra.h:11: /// \brief Declares command line flags primarily used
for
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceCompiler.cpp
File src/IceCompiler.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCompiler.cpp#newcode14
src/IceCompiler.cpp:14: /// routines to parse a textual bitcode file into LLVM
IR and then convert
Reflow to 80-col.  (Pretty sure LLVM can fit at the end of this line.)

https://codereview.chromium.org/1488913002/diff/1/src/IceELFStreamer.h
File src/IceELFStreamer.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceELFStreamer.h#newcode11
src/IceELFStreamer.h:11: /// \brief Interface for serializing bits for common
ELF types (words, extended words,
80-col

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.cpp
File src/IceGlobalContext.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.cpp#ne...
src/IceGlobalContext.cpp:11: /// \brief Defines aspects of the compilation that
persist across
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.h
File src/IceGlobalContext.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.h#newc...
src/IceGlobalContext.h:11: /// \brief Declares aspects of the compilation that
persist across
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalInits.cpp
File src/IceGlobalInits.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalInits.cpp#newc...
src/IceGlobalInits.cpp:11: /// \brief Implements the notion of function
declarations, global
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceInst.h
File src/IceInst.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceInst.h#newcode13
src/IceInst.h:13: /// These represent the high-level Vanilla ICE instructions
and map roughly 1:1 to
80-col

https://codereview.chromium.org/1488913002/diff/1/src/IceInstMIPS32.cpp
File src/IceInstMIPS32.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceInstMIPS32.cpp#newco...
src/IceInstMIPS32.cpp:11: /// \brief Implements the InstMips32 and OperandMips32
classes,
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceInstVarIter.h
File src/IceInstVarIter.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceInstVarIter.h#newcode11
src/IceInstVarIter.h:11: /// \brief Defines a common pattern for iterating over
the variables
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceInstX8632.h
File src/IceInstX8632.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceInstX8632.h#newcode13
src/IceInstX8632.h:13: /// Subzero has been modified to use templates for X86
instructions, so all those
80-col

https://codereview.chromium.org/1488913002/diff/1/src/IceLiveness.h
File src/IceLiveness.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceLiveness.h#newcode11
src/IceLiveness.h:11: /// \brief Declares the Liveness and LivenessNode classes,
which are
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceOperand.cpp
File src/IceOperand.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceOperand.cpp#newcode11
src/IceOperand.cpp:11: /// \brief Implements the Operand class and its
target-independent
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceSwitchLowering.h
File src/IceSwitchLowering.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceSwitchLowering.h#new...
src/IceSwitchLowering.h:11: /// \brief Contains helpers for switch lowering.
Maybe just "Helpers for switch lowering."

https://codereview.chromium.org/1488913002/diff/1/src/IceTLS.h
File src/IceTLS.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTLS.h#newcode11
src/IceTLS.h:11: /// \brief Defines macros for working around the lack of
support for
Does thread_local fit on this line within 80-cols?

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX8664.cpp
File src/IceTargetLoweringX8664.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX8664....
src/IceTargetLoweringX8664.cpp:11: /// \brief implements the TargetLoweringX8664
class, which consists almost
capitalize Implements

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Base.h
File src/IceTargetLoweringX86Base.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Bas...
src/IceTargetLoweringX86Base.h:11: /// \brief Declares the TargetLoweringX86
template class, which implements
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Reg...
File src/IceTargetLoweringX86RegClass.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Reg...
src/IceTargetLoweringX86RegClass.h:11: /// \brief eclares the X86 register class
extensions.
Declares

https://codereview.chromium.org/1488913002/diff/1/src/IceTranslator.cpp
File src/IceTranslator.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTranslator.cpp#newco...
src/IceTranslator.cpp:11: /// \brief Defines the general driver class for
translating ICE to machine
Reflow to 80-col.  (I think)

https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.cpp
File src/IceTypeConverter.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.cpp#ne...
src/IceTypeConverter.cpp:11: /// \brief Implements how to convert LLVM types to
ICE types, and ICE types
Reflow to 80-col.

But maybe it's better stated like this:

Implements conversion between LLVM and ICE types.

https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.h
File src/IceTypeConverter.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.h#newc...
src/IceTypeConverter.h:11: /// \brief Defines how to convert LLVM types to ICE
types, and ICE types to
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceTypes.h
File src/IceTypes.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTypes.h#newcode11
src/IceTypes.h:11: /// \brief Declares a few properties of the primitive types
allowed in
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/1/src/IceUtils.h
File src/IceUtils.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceUtils.h#newcode11
src/IceUtils.h:11: /// \brief Declares some utility functions.
Probably "defines" instead of "declares" since it's all inline.

https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.cpp#new...
src/PNaClTranslator.cpp:11: /// \brief Implements the PNaCl bitcode file to Ice,
to machine code
How about:

Implements the interface for translation from PNaCl bitcode files to ICE to
machine code.

https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.h
File src/PNaClTranslator.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.h#newco...
src/PNaClTranslator.h:11: /// \brief Declares the PNaCl bitcode file to ICE, to
machine code
How about:

Declares the interface for translation from PNaCl bitcode files to ICE to
machine code.

[rkotlerimgtec, 2015-12-01 22:03:09]

I think "declares" is trying to match the C++ "declare" but then
"implementation" should have been "defines".

I think more appropriate would be the "specifies" and "implements".

It's looser and with inlining there are declarations of methods in class
declarations.

But we could stick with "declares" and "defines" with "declares" in the .h
and
"defines" in the .cpp

On Tue, Dec 1, 2015 at 10:41 AM, <stichnot@chromium.org> wrote:

> Thanks!
>
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceAPFloat.h
> File src/IceAPFloat.h (right):
>
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceAPFloat.h#newcode11
> src/IceAPFloat.h:11: /// \brief Implements a class to represent float
> and double
> Reflow to 80-col
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceAPInt.h
> File src/IceAPInt.h (right):
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceAPInt.h#newcode11
> src/IceAPInt.h:11: /// \brief Implements a class to represent 64 bit
> integer constant
> Reflow to 80-col
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerARM32.h
> File src/IceAssemblerARM32.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerARM32.h#new...
> src/IceAssemblerARM32.h:19: /// \brief Implements the Assembler class
> for ARM32.
> The .cpp file also says "implements".  In some other cases, the .h file
> says "declares" while the .cpp file says "implements".
>
> Do we / should we have a rule for "implements" vs "declares"?
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerX86Base.h
> File src/IceAssemblerX86Base.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerX86Base.h#n...
> src/IceAssemblerX86Base.h:19: /// \brief Defines the AssemblerX86
> template class for x86, the base of all
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.cpp
> File src/IceCfg.cpp (right):
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.cpp#newcode11
> src/IceCfg.cpp:11: /// \brief Implements the Cfg class, including
> constant pool management.
> I don't think Cfg has anything to do with constant pool management.  I
> wonder if that was a copy&paste mistake.  Just remove that clause?
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.h
> File src/IceCfg.h (right):
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.h#newcode11
> src/IceCfg.h:11: /// \brief Declares the Cfg class, which represents the
> control flow graph
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.cpp
> File src/IceCfgNode.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.cpp#newcode11
> src/IceCfgNode.cpp:11: /// \brief Implements the CfgNode class,
> including the complexities
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.h
> File src/IceCfgNode.h (right):
>
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.h#newcode11
> src/IceCfgNode.h:11: /// \brief Declares the CfgNode class, which
> represents a single basic block
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceClFlagsExtra.h
> File src/IceClFlagsExtra.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceClFlagsExtra.h#newco...
> src/IceClFlagsExtra.h:11: /// \brief Declares command line flags
> primarily used for
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceCompiler.cpp
> File src/IceCompiler.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceCompiler.cpp#newcode14
> src/IceCompiler.cpp:14: /// routines to parse a textual bitcode file
> into LLVM IR and then convert
> Reflow to 80-col.  (Pretty sure LLVM can fit at the end of this line.)
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceELFStreamer.h
> File src/IceELFStreamer.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceELFStreamer.h#newcode11
> src/IceELFStreamer.h:11: /// \brief Interface for serializing bits for
> common ELF types (words, extended words,
> 80-col
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.cpp
> File src/IceGlobalContext.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.cpp#ne...
> src/IceGlobalContext.cpp:11: /// \brief Defines aspects of the
> compilation that persist across
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.h
> File src/IceGlobalContext.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.h#newc...
> src/IceGlobalContext.h:11: /// \brief Declares aspects of the
> compilation that persist across
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalInits.cpp
> File src/IceGlobalInits.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalInits.cpp#newc...
> src/IceGlobalInits.cpp:11: /// \brief Implements the notion of function
> declarations, global
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceInst.h
> File src/IceInst.h (right):
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceInst.h#newcode13
> src/IceInst.h:13: /// These represent the high-level Vanilla ICE
> instructions and map roughly 1:1 to
> 80-col
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceInstMIPS32.cpp
> File src/IceInstMIPS32.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceInstMIPS32.cpp#newco...
> src/IceInstMIPS32.cpp:11: /// \brief Implements the InstMips32 and
> OperandMips32 classes,
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceInstVarIter.h
> File src/IceInstVarIter.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceInstVarIter.h#newcode11
> src/IceInstVarIter.h:11: /// \brief Defines a common pattern for
> iterating over the variables
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceInstX8632.h
> File src/IceInstX8632.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceInstX8632.h#newcode13
> src/IceInstX8632.h:13: /// Subzero has been modified to use templates
> for X86 instructions, so all those
> 80-col
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceLiveness.h
> File src/IceLiveness.h (right):
>
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceLiveness.h#newcode11
> src/IceLiveness.h:11: /// \brief Declares the Liveness and LivenessNode
> classes, which are
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceOperand.cpp
> File src/IceOperand.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceOperand.cpp#newcode11
> src/IceOperand.cpp:11: /// \brief Implements the Operand class and its
> target-independent
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceSwitchLowering.h
> File src/IceSwitchLowering.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceSwitchLowering.h#new...
> src/IceSwitchLowering.h:11: /// \brief Contains helpers for switch
> lowering.
> Maybe just "Helpers for switch lowering."
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceTLS.h
> File src/IceTLS.h (right):
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceTLS.h#newcode11
> src/IceTLS.h:11: /// \brief Defines macros for working around the lack
> of support for
> Does thread_local fit on this line within 80-cols?
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX8664.cpp
> File src/IceTargetLoweringX8664.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX8664....
> src/IceTargetLoweringX8664.cpp:11: /// \brief implements the
> TargetLoweringX8664 class, which consists almost
> capitalize Implements
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Base.h
> File src/IceTargetLoweringX86Base.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Bas...
> src/IceTargetLoweringX86Base.h:11: /// \brief Declares the
> TargetLoweringX86 template class, which implements
> Reflow to 80-col.
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Reg...
> File src/IceTargetLoweringX86RegClass.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Reg...
> src/IceTargetLoweringX86RegClass.h:11: /// \brief eclares the X86
> register class extensions.
> Declares
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceTranslator.cpp
> File src/IceTranslator.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTranslator.cpp#newco...
> src/IceTranslator.cpp:11: /// \brief Defines the general driver class
> for translating ICE to machine
> Reflow to 80-col.  (I think)
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.cpp
> File src/IceTypeConverter.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.cpp#ne...
> src/IceTypeConverter.cpp:11: /// \brief Implements how to convert LLVM
> types to ICE types, and ICE types
> Reflow to 80-col.
>
> But maybe it's better stated like this:
>
> Implements conversion between LLVM and ICE types.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.h
> File src/IceTypeConverter.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.h#newc...
> src/IceTypeConverter.h:11: /// \brief Defines how to convert LLVM types
> to ICE types, and ICE types to
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceTypes.h
> File src/IceTypes.h (right):
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceTypes.h#newcode11
> src/IceTypes.h:11: /// \brief Declares a few properties of the primitive
> types allowed in
> Reflow to 80-col.
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceUtils.h
> File src/IceUtils.h (right):
>
> https://codereview.chromium.org/1488913002/diff/1/src/IceUtils.h#newcode11
> src/IceUtils.h:11: /// \brief Declares some utility functions.
> Probably "defines" instead of "declares" since it's all inline.
>
> https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.cpp
> File src/PNaClTranslator.cpp (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.cpp#new...
> src/PNaClTranslator.cpp:11: /// \brief Implements the PNaCl bitcode file
> to Ice, to machine code
> How about:
>
> Implements the interface for translation from PNaCl bitcode files to ICE
> to machine code.
>
> https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.h
> File src/PNaClTranslator.h (right):
>
>
>
https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.h#newco...
> src/PNaClTranslator.h:11: /// \brief Declares the PNaCl bitcode file to
> ICE, to machine code
> How about:
>
> Declares the interface for translation from PNaCl bitcode files to ICE
> to machine code.
>
> https://codereview.chromium.org/1488913002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/d/optout.

[rkotlerimgtec, 2015-12-02 01:32:49]

https://codereview.chromium.org/1488913002/diff/1/src/IceAPFloat.h
File src/IceAPFloat.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceAPFloat.h#newcode11
src/IceAPFloat.h:11: /// \brief Implements a class to represent float and double
On 2015/12/01 18:41:15, stichnot wrote:
> Reflow to 80-col

this file has been deleted

https://codereview.chromium.org/1488913002/diff/1/src/IceAPInt.h
File src/IceAPInt.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceAPInt.h#newcode11
src/IceAPInt.h:11: /// \brief Implements a class to represent 64 bit integer
constant
On 2015/12/01 18:41:15, stichnot wrote:
> Reflow to 80-col

this file has been deleted

https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerARM32.h
File src/IceAssemblerARM32.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerARM32.h#new...
src/IceAssemblerARM32.h:19: /// \brief Implements the Assembler class for ARM32.
On 2015/12/01 18:41:15, stichnot wrote:
> The .cpp file also says "implements".  In some other cases, the .h file says
> "declares" while the .cpp file says "implements".
> 
> Do we / should we have a rule for "implements" vs "declares"?

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerX86Base.h
File src/IceAssemblerX86Base.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceAssemblerX86Base.h#n...
src/IceAssemblerX86Base.h:19: /// \brief Defines the AssemblerX86 template class
for x86, the base of all
On 2015/12/01 18:41:15, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.cpp
File src/IceCfg.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.cpp#newcode11
src/IceCfg.cpp:11: /// \brief Implements the Cfg class, including constant pool
management.
On 2015/12/01 18:41:16, stichnot wrote:
> I don't think Cfg has anything to do with constant pool management.  I wonder
if
> that was a copy&paste mistake.  Just remove that clause?

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.h
File src/IceCfg.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCfg.h#newcode11
src/IceCfg.h:11: /// \brief Declares the Cfg class, which represents the control
flow graph
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.cpp
File src/IceCfgNode.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.cpp#newcode11
src/IceCfgNode.cpp:11: /// \brief Implements the CfgNode class, including the
complexities
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.h
File src/IceCfgNode.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCfgNode.h#newcode11
src/IceCfgNode.h:11: /// \brief Declares the CfgNode class, which represents a
single basic block
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceClFlagsExtra.h
File src/IceClFlagsExtra.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceClFlagsExtra.h#newco...
src/IceClFlagsExtra.h:11: /// \brief Declares command line flags primarily used
for
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceCompiler.cpp
File src/IceCompiler.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceCompiler.cpp#newcode14
src/IceCompiler.cpp:14: /// routines to parse a textual bitcode file into LLVM
IR and then convert
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.  (Pretty sure LLVM can fit at the end of this line.)

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceELFStreamer.h
File src/IceELFStreamer.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceELFStreamer.h#newcode11
src/IceELFStreamer.h:11: /// \brief Interface for serializing bits for common
ELF types (words, extended words,
On 2015/12/01 18:41:16, stichnot wrote:
> 80-col

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.cpp
File src/IceGlobalContext.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.cpp#ne...
src/IceGlobalContext.cpp:11: /// \brief Defines aspects of the compilation that
persist across
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.h
File src/IceGlobalContext.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalContext.h#newc...
src/IceGlobalContext.h:11: /// \brief Declares aspects of the compilation that
persist across
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalInits.cpp
File src/IceGlobalInits.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceGlobalInits.cpp#newc...
src/IceGlobalInits.cpp:11: /// \brief Implements the notion of function
declarations, global
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceInst.h
File src/IceInst.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceInst.h#newcode13
src/IceInst.h:13: /// These represent the high-level Vanilla ICE instructions
and map roughly 1:1 to
On 2015/12/01 18:41:16, stichnot wrote:
> 80-col

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceInstMIPS32.cpp
File src/IceInstMIPS32.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceInstMIPS32.cpp#newco...
src/IceInstMIPS32.cpp:11: /// \brief Implements the InstMips32 and OperandMips32
classes,
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceInstVarIter.h
File src/IceInstVarIter.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceInstVarIter.h#newcode11
src/IceInstVarIter.h:11: /// \brief Defines a common pattern for iterating over
the variables
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceInstX8632.h
File src/IceInstX8632.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceInstX8632.h#newcode13
src/IceInstX8632.h:13: /// Subzero has been modified to use templates for X86
instructions, so all those
On 2015/12/01 18:41:16, stichnot wrote:
> 80-col

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceLiveness.h
File src/IceLiveness.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceLiveness.h#newcode11
src/IceLiveness.h:11: /// \brief Declares the Liveness and LivenessNode classes,
which are
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceOperand.cpp
File src/IceOperand.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceOperand.cpp#newcode11
src/IceOperand.cpp:11: /// \brief Implements the Operand class and its
target-independent
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceSwitchLowering.h
File src/IceSwitchLowering.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceSwitchLowering.h#new...
src/IceSwitchLowering.h:11: /// \brief Contains helpers for switch lowering.
On 2015/12/01 18:41:16, stichnot wrote:
> Maybe just "Helpers for switch lowering."

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceTLS.h
File src/IceTLS.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTLS.h#newcode11
src/IceTLS.h:11: /// \brief Defines macros for working around the lack of
support for
On 2015/12/01 18:41:16, stichnot wrote:
> Does thread_local fit on this line within 80-cols?

no

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX8664.cpp
File src/IceTargetLoweringX8664.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX8664....
src/IceTargetLoweringX8664.cpp:11: /// \brief implements the TargetLoweringX8664
class, which consists almost
On 2015/12/01 18:41:16, stichnot wrote:
> capitalize Implements

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Base.h
File src/IceTargetLoweringX86Base.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Bas...
src/IceTargetLoweringX86Base.h:11: /// \brief Declares the TargetLoweringX86
template class, which implements
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Reg...
File src/IceTargetLoweringX86RegClass.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTargetLoweringX86Reg...
src/IceTargetLoweringX86RegClass.h:11: /// \brief eclares the X86 register class
extensions.
On 2015/12/01 18:41:16, stichnot wrote:
> Declares

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceTranslator.cpp
File src/IceTranslator.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTranslator.cpp#newco...
src/IceTranslator.cpp:11: /// \brief Defines the general driver class for
translating ICE to machine
On 2015/12/01 18:41:16, stichnot wrote:
> Reflow to 80-col.  (I think)

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.cpp
File src/IceTypeConverter.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.cpp#ne...
src/IceTypeConverter.cpp:11: /// \brief Implements how to convert LLVM types to
ICE types, and ICE types
On 2015/12/01 18:41:17, stichnot wrote:
> Reflow to 80-col.
> 
> But maybe it's better stated like this:
> 
> Implements conversion between LLVM and ICE types.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.h
File src/IceTypeConverter.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTypeConverter.h#newc...
src/IceTypeConverter.h:11: /// \brief Defines how to convert LLVM types to ICE
types, and ICE types to
On 2015/12/01 18:41:17, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceTypes.h
File src/IceTypes.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceTypes.h#newcode11
src/IceTypes.h:11: /// \brief Declares a few properties of the primitive types
allowed in
On 2015/12/01 18:41:17, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/IceUtils.h
File src/IceUtils.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/IceUtils.h#newcode11
src/IceUtils.h:11: /// \brief Declares some utility functions.
On 2015/12/01 18:41:17, stichnot wrote:
> Probably "defines" instead of "declares" since it's all inline.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.cpp#new...
src/PNaClTranslator.cpp:11: /// \brief Implements the PNaCl bitcode file to Ice,
to machine code
On 2015/12/01 18:41:17, stichnot wrote:
> How about:
> 
> Implements the interface for translation from PNaCl bitcode files to ICE to
> machine code.

Done.

https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.h
File src/PNaClTranslator.h (right):

https://codereview.chromium.org/1488913002/diff/1/src/PNaClTranslator.h#newco...
src/PNaClTranslator.h:11: /// \brief Declares the PNaCl bitcode file to ICE, to
machine code
On 2015/12/01 18:41:17, stichnot wrote:
> How about:
> 
> Declares the interface for translation from PNaCl bitcode files to ICE to
> machine code.

Done.

[rkotlerimgtec, 2015-12-02 02:34:18]

I'm using doxygen 1.8.10 which is the latest.

I think that the brief documentation for Subzero may have worked ok on much
earlier versions of Doxygen.

LLVM Doxygen on the web is using 1.7.6.1.

LLVM Doxygen is also missing the brief descriptions for almost all its
files but for a different reason, just the lack of the /// (the have //).
http://llvm.org/docs/doxygen/html/files.html

But their ones that worked did not have the \brief but just seemed to parse
the first sentence to get that (up to the period or period space).

When I try that with 1.8.10, I get the problem we were fixing and the ones
that were working for Subzero before my patch all had the \brief directive.

So I think my patch is correct and not overkill but it's possible that
some of it was not necessary on earlier versions of Doxygen.

I'm just adding this by way of explanation of how we seemed to have lost that
info when obviously someone else that originally did this had to know about all
of this since they were championing the tool. They may have
just copied what LLVM was doing from the places that it worked for them.

I just started to learn Doxygen right before I did my first patch.

My reading of the documentation is that you need the /brief directive when
you have the /file directive.

It's also possible that the files that have doc in llvm are getting the
information in a different way by using a structure comment in a .cpp 
somewhere but it seems unlikely.

[rkotlerimgtec, 2015-12-02 03:01:47]

I did some more digging and there were some Doxygen changes in how brief
descriptions worked and were separated from detailed.

I think what I did was correct but just adding some information by way of
explanation of why it is not working before my patch but probably did work
at some earlier point in time.

There are some configuration options for BRIEF but that do not seem to be
relevant to this issue. REPEAT_BRIEF, ABBREVIATE_BRIEF.

I will try and understand this tool better so we can use it in the easiest way
and get the best results.


# If the BRIEF_MEMBER_DESC tag is set to YES doxygen will include brief member
# descriptions after the members that are listed in the file and class
# documentation (similar to Javadoc). Set to NO to disable this.
# The default value is: YES.

BRIEF_MEMBER_DESC      = YES

# If the REPEAT_BRIEF tag is set to YES doxygen will prepend the brief
# description of a member or function before the detailed description
#
# Note: If both HIDE_UNDOC_MEMBERS and BRIEF_MEMBER_DESC are set to NO, the
# brief descriptions will be completely suppressed.
# The default value is: YES.

REPEAT_BRIEF           = YES

# This tag implements a quasi-intelligent brief description abbreviator that is
# used to form the text in various listings. Each string in this list, if found
# as the leading text of the brief description, will be stripped from the text
# and the result, after processing the whole list, is used as the annotated
# text. Otherwise, the brief description is used as-is. If left blank, the
# following values are used ($name is automatically replaced with the name of
# the entity):The $name class, The $name widget, The $name file, is, provides,
# specifies, contains, represents, a, an and the.

ABBREVIATE_BRIEF       =

[rkotlerimgtec, 2015-12-02 03:17:28]

For sure with my patch, everything looks great on the file descriptions when
using the latest version of doxygen. Did not intend to confuse the issue here;
was just trying to give some explanation of why it might have basically worked
in the past.

Without my patch, we get almost no file descriptions and the only ones we do get
are from files that were done like my patch is.

[Jim Stichnoth, 2015-12-02 21:50:38]

https://codereview.chromium.org/1488913002/diff/20001/src/IceAssemblerMIPS32.h
File src/IceAssemblerMIPS32.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceAssemblerMIPS32....
src/IceAssemblerMIPS32.h:19: /// \brief Implements the Assembler class for
MIPS32.
Declares, not implements, right?

(For now, I guess it does both, but in the future there will be a .cpp file as
well.)

https://codereview.chromium.org/1488913002/diff/20001/src/IceAssemblerX86Base...
File src/IceAssemblerX86BaseImpl.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceAssemblerX86Base...
src/IceAssemblerX86BaseImpl.h:18: /// \brief  Implements the AssemblerX86Base
template class, which is the
Single space after \brief.

Also, reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/20001/src/IceBrowserCompileSe...
File src/IceBrowserCompileServer.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceBrowserCompileSe...
src/IceBrowserCompileServer.h:11: /// \brief Declares the browser-specific
compile server.
I think this should say "browser-based" like it does in the .cpp file.

https://codereview.chromium.org/1488913002/diff/20001/src/IceCfg.h
File src/IceCfg.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceCfg.h#newcode11
src/IceCfg.h:11: /// \brief Declares the Cfg class, which represents the control
flow graph the
s/graph the/graph and the/

https://codereview.chromium.org/1488913002/diff/20001/src/IceClFlags.cpp
File src/IceClFlags.cpp (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceClFlags.cpp#newc...
src/IceClFlags.cpp:13: /// This currently relies on llvm::cl to parse. In the
future, the minimal
Reflow to 80-col ("build" should fit on this line).

https://codereview.chromium.org/1488913002/diff/20001/src/IceELFStreamer.h
File src/IceELFStreamer.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceELFStreamer.h#ne...
src/IceELFStreamer.h:12: /// words, etc.), based on the ELF Class.
Probably "class" shouldn't be capitalized.

https://codereview.chromium.org/1488913002/diff/20001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:11: /// \brief Declares the InstARM32 and OperandARM32
classes and their
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/20001/src/IceInstX8664.h
File src/IceInstX8664.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceInstX8664.h#newc...
src/IceInstX8664.h:12: ///  src/IceInstX86Base.)
/// src/
- instead of -
///  src/

https://codereview.chromium.org/1488913002/diff/20001/src/IceIntrinsics.cpp
File src/IceIntrinsics.cpp (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceIntrinsics.cpp#n...
src/IceIntrinsics.cpp:11: /// \brief Implements the Intrinsics utilities for
matching and then
Reflow to 80-col.

https://codereview.chromium.org/1488913002/diff/20001/src/IceTypeConverter.h
File src/IceTypeConverter.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceTypeConverter.h#...
src/IceTypeConverter.h:11: /// \brief Defines how to convert LLVM types to ICE
types, and ICE types to
Reflow to 80-col.

[rkotlerimgtec, 2015-12-02 23:58:14]

https://codereview.chromium.org/1488913002/diff/20001/src/IceAssemblerMIPS32.h
File src/IceAssemblerMIPS32.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceAssemblerMIPS32....
src/IceAssemblerMIPS32.h:19: /// \brief Implements the Assembler class for
MIPS32.
On 2015/12/02 21:50:38, stichnot wrote:
> Declares, not implements, right?
> 
> (For now, I guess it does both, but in the future there will be a .cpp file as
> well.)

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceAssemblerX86Base...
File src/IceAssemblerX86BaseImpl.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceAssemblerX86Base...
src/IceAssemblerX86BaseImpl.h:18: /// \brief  Implements the AssemblerX86Base
template class, which is the
On 2015/12/02 21:50:38, stichnot wrote:
> Single space after \brief.
> 
> Also, reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceBrowserCompileSe...
File src/IceBrowserCompileServer.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceBrowserCompileSe...
src/IceBrowserCompileServer.h:11: /// \brief Declares the browser-specific
compile server.
On 2015/12/02 21:50:38, stichnot wrote:
> I think this should say "browser-based" like it does in the .cpp file.

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceCfg.h
File src/IceCfg.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceCfg.h#newcode11
src/IceCfg.h:11: /// \brief Declares the Cfg class, which represents the control
flow graph the
On 2015/12/02 21:50:38, stichnot wrote:
> s/graph the/graph and the/

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceClFlags.cpp
File src/IceClFlags.cpp (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceClFlags.cpp#newc...
src/IceClFlags.cpp:13: /// This currently relies on llvm::cl to parse. In the
future, the minimal
On 2015/12/02 21:50:38, stichnot wrote:
> Reflow to 80-col ("build" should fit on this line).

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceELFStreamer.h
File src/IceELFStreamer.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceELFStreamer.h#ne...
src/IceELFStreamer.h:12: /// words, etc.), based on the ELF Class.
On 2015/12/02 21:50:38, stichnot wrote:
> Probably "class" shouldn't be capitalized.

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceInstARM32.h
File src/IceInstARM32.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceInstARM32.h#newc...
src/IceInstARM32.h:11: /// \brief Declares the InstARM32 and OperandARM32
classes and their
On 2015/12/02 21:50:38, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceInstX8664.h
File src/IceInstX8664.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceInstX8664.h#newc...
src/IceInstX8664.h:12: ///  src/IceInstX86Base.)
On 2015/12/02 21:50:38, stichnot wrote:
> /// src/
> - instead of -
> ///  src/

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceIntrinsics.cpp
File src/IceIntrinsics.cpp (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceIntrinsics.cpp#n...
src/IceIntrinsics.cpp:11: /// \brief Implements the Intrinsics utilities for
matching and then
On 2015/12/02 21:50:38, stichnot wrote:
> Reflow to 80-col.

Done.

https://codereview.chromium.org/1488913002/diff/20001/src/IceTypeConverter.h
File src/IceTypeConverter.h (right):

https://codereview.chromium.org/1488913002/diff/20001/src/IceTypeConverter.h#...
src/IceTypeConverter.h:11: /// \brief Defines how to convert LLVM types to ICE
types, and ICE types to
On 2015/12/02 21:50:38, stichnot wrote:
> Reflow to 80-col.

Done.

[Jim Stichnoth, 2015-12-03 00:26:42]

lgtm

[Jim Stichnoth, 2015-12-03 00:52:49]

Description was changed from

==========
fix doxygen brief in subzero header files

There is a consistent mistake in most of the file level subzero doxygen
comments which stems from a problem with brief.

Basically, there are two types of documentation in Doxygen, inline and
structural documentation. File level documentation is always structural
because inline documentation must either come before or after the entity
and for a file that would be impossible.

When you have structure documentation, the brief and detailed is
handled differently and you have to explicity put the \brief qualifier in.

We were only doing this for a few files so when you looked the doxygen
for the source directory, the description was blank for almost all of them.
If you build docs with this patch and look at the "files" and src
directory, you will see all the brief descriptions now.

In addition I deleted the text "This file ..." from the 
beginning of all the file desriptions because it's redundant when you look at
the doxygen output and adds
no information but takes up space in the descripton.
BUG=
==========

to

==========
fix doxygen brief in subzero header files

There is a consistent mistake in most of the file level subzero doxygen
comments which stems from a problem with brief.

Basically, there are two types of documentation in Doxygen, inline and
structural documentation. File level documentation is always structural
because inline documentation must either come before or after the entity
and for a file that would be impossible.

When you have structure documentation, the brief and detailed is
handled differently and you have to explicity put the \brief qualifier in.

We were only doing this for a few files so when you looked the doxygen
for the source directory, the description was blank for almost all of them.
If you build docs with this patch and look at the "files" and src
directory, you will see all the brief descriptions now.

In addition I deleted the text "This file ..." from the
beginning of all the file desriptions because it's redundant when you look at
the doxygen output and adds
no information but takes up space in the descripton.
BUG=
R=stichnot@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[Jim Stichnoth, 2015-12-03 00:52:51]

Committed patchset #3 (id:40001) manually as
92a6e5b08ec68e7076d637ebc680da2fcc346a00 (presubmit successful).