[NFC] Remove doxygen brief tag from BasicBlock.h

[mattd]

Summary

Documentation is built using the AutoBrief configuration option in docs/doxygen.cfg.in. This change removes the redundant brief tags from BasicBlock.h. I'm happy to write a sed script and remove all \brief tags as a separate commit later.

[aprantl]

thanks!

[aprantl]

BasicBlock.h

138

You might want to reflow the comments afterwards (Esc q / M-x fill-paragraph in emacs).

[mattd]

• Removed brief doxygen tag.
• Reflowed the comment text of the paragraph containing the brief tag.
• Fixed a typo.

[MatzeB]

I am pretty sure this is not correct: Autobrief is stupid and stops at the first dot or at the line ending AFAIK. This means when the first sentence stretches over more than 1 line you still need need \brief. So a bunch of the changes here appear invalid to me, please check the doxygen output and revert if necessary.

[mattd]

I diffed the two outputs and also pulled up the text in a browser. I do not see any significant changes in browser, and there wasn't. However, I am happy to revert if any noticeable changes are witnessed. I see multiline/paragraph text in the output.

[MatzeB]

In D45708#1070667, @mattd wrote:

I diffed the two outputs and also pulled up the text in a browser. I do not see any significant changes in browser, and there wasn't. However, I am happy to revert if any noticeable changes are witnessed. I see multiline/paragraph text in the output.

Does that mean the doxygen documentation is just wrong?

https://www.stack.nl/~dimitri/doxygen/manual/config.html#cfg_javadoc_autobrief claims:

If the JAVADOC_AUTOBRIEF tag is set to YES then doxygen will interpret the first line (until the first dot) of a Javadoc-style comment as the brief description. > If set to NO, the Javadoc-style will behave just like regular Qt-style comments (thus requiring an explicit @brief command for a brief description.)

or does doxygen have a different notion of a what “first line” means?

[mattd]

In D45708#1070682, @MatzeB wrote:

In D45708#1070667, @mattd wrote:

I diffed the two outputs and also pulled up the text in a browser. I do not see any significant changes in browser, and there wasn't. However, I am happy to revert if any noticeable changes are witnessed. I see multiline/paragraph text in the output.

Does that mean the doxygen documentation is just wrong?

https://www.stack.nl/~dimitri/doxygen/manual/config.html#cfg_javadoc_autobrief claims:

If the JAVADOC_AUTOBRIEF tag is set to YES then doxygen will interpret the first line (until the first dot) of a Javadoc-style comment as the brief description. > If set to NO, the Javadoc-style will behave just like regular Qt-style comments (thus requiring an explicit @brief command for a brief description.)

or does doxygen have a different notion of a what “first line” means?

What I am seeing, is what I was expecting, the first line is up until the first dot, or to the first blank comment line. The definition you reference seems incomplete; Mainly, the end of brief appears to be either either the first . in the comment or the first blank comment line.

BasicBlock.h

 class Function;
 class LandingPadInst;
 class LLVMContext;
 class Module;
 class PHINode;
 class TerminatorInst;
 class ValueSymbolTable;
 
-/// \brief LLVM Basic Block Representation
+/// LLVM Basic Block Representation
 ///
 /// This represents a single basic block in LLVM. A basic block is simply a
 /// container of instructions that execute sequentially. Basic blocks are Values
 /// because they are referenced by instructions such as branches and switch
 /// tables. The type of a BasicBlock is "Type::LabelTy" because the basic block
 /// represents a label to which a branch can jump.
 ///
 /// A well formed basic block is formed of a list of non-terminating
private:
   friend class BlockAddress;
   friend class SymbolTableListTraits<BasicBlock>;
 
   InstListType InstList;
   Function *Parent;
 
   void setParent(Function *parent);
 
-  /// \brief Constructor.
+  /// Constructor.
   ///
   /// If the function parameter is specified, the basic block is automatically
   /// inserted at either the end of the function (if InsertBefore is null), or
   /// before the specified basic block.
   explicit BasicBlock(LLVMContext &C, const Twine &Name = "",
                       Function *Parent = nullptr,
                       BasicBlock *InsertBefore = nullptr);
 
 public:
   BasicBlock(const BasicBlock &) = delete;
   BasicBlock &operator=(const BasicBlock &) = delete;
   ~BasicBlock();
 
-  /// \brief Get the context in which this basic block lives.
+  /// Get the context in which this basic block lives.
   LLVMContext &getContext() const;
 
   /// Instruction iterators...
   using iterator = InstListType::iterator;
   using const_iterator = InstListType::const_iterator;
   using reverse_iterator = InstListType::reverse_iterator;
   using const_reverse_iterator = InstListType::const_reverse_iterator;
 
-  /// \brief Creates a new BasicBlock.
+  /// Creates a new BasicBlock.
   ///
   /// If the Parent parameter is specified, the basic block is automatically
   /// inserted at either the end of the function (if InsertBefore is 0), or
   /// before the specified basic block.
   static BasicBlock *Create(LLVMContext &Context, const Twine &Name = "",
                             Function *Parent = nullptr,
                             BasicBlock *InsertBefore = nullptr) {
     return new BasicBlock(Context, Name, Parent, InsertBefore);
   }
 
-  /// \brief Return the enclosing method, or null if none.
+  /// Return the enclosing method, or null if none.
   const Function *getParent() const { return Parent; }
         Function *getParent()       { return Parent; }
 
-  /// \brief Return the module owning the function this basic block belongs to,
+  /// Return the module owning the function this basic block belongs to,
   /// or nullptr it the function does not have a module.
   ///
   /// Note: this is undefined behavior if the block does not have a parent.
   const Module *getModule() const;
   Module *getModule() {
     return const_cast<Module *>(
                             static_cast<const BasicBlock *>(this)->getModule());
   }
 
-  /// \brief Returns the terminator instruction if the block is well formed or
+  /// Returns the terminator instruction if the block is well formed or
   /// null if the block is not well formed.
   const TerminatorInst *getTerminator() const LLVM_READONLY;
   TerminatorInst *getTerminator() {
     return const_cast<TerminatorInst *>(
                         static_cast<const BasicBlock *>(this)->getTerminator());
   }
 
-  /// \brief Returns the call instruction calling @llvm.experimental.deoptimize
+  /// Returns the call instruction calling @llvm.experimental.deoptimize
   /// prior to the terminating return instruction of this basic block, if such a
   /// call is present.  Otherwise, returns null.
   const CallInst *getTerminatingDeoptimizeCall() const;
   CallInst *getTerminatingDeoptimizeCall() {
     return const_cast<CallInst *>(
          static_cast<const BasicBlock *>(this)->getTerminatingDeoptimizeCall());
   }
 
-  /// \brief Returns the call instruction marked 'musttail' prior to the
+  /// Returns the call instruction marked 'musttail' prior to the
   /// terminating return instruction of this basic block, if such a call is
   /// present.  Otherwise, returns null.
   const CallInst *getTerminatingMustTailCall() const;
   CallInst *getTerminatingMustTailCall() {
     return const_cast<CallInst *>(
            static_cast<const BasicBlock *>(this)->getTerminatingMustTailCall());
   }
 
-  /// \brief Returns a pointer to the first instruction in this block that is
+  /// Returns a pointer to the first instruction in this block that is
   /// not a PHINode instruction.
   ///
   /// When adding instructions to the beginning of the basic block, they should
   /// be added before the returned value, not before the first instruction,
   /// which might be PHI. Returns 0 is there's no non-PHI instruction.
   const Instruction* getFirstNonPHI() const;
   Instruction* getFirstNonPHI() {
     return const_cast<Instruction *>(
                        static_cast<const BasicBlock *>(this)->getFirstNonPHI());
   }
 
-  /// \brief Returns a pointer to the first instruction in this block that is not
+  /// Returns a pointer to the first instruction in this block that is not
   /// a PHINode or a debug intrinsic.
   const Instruction* getFirstNonPHIOrDbg() const;
   Instruction* getFirstNonPHIOrDbg() {
     return const_cast<Instruction *>(
                   static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbg());
   }
 
-  /// \brief Returns a pointer to the first instruction in this block that is not
+  /// Returns a pointer to the first instruction in this block that is not
   /// a PHINode, a debug intrinsic, or a lifetime intrinsic.
   const Instruction* getFirstNonPHIOrDbgOrLifetime() const;
   Instruction* getFirstNonPHIOrDbgOrLifetime() {
     return const_cast<Instruction *>(
         static_cast<const BasicBlock *>(this)->getFirstNonPHIOrDbgOrLifetime());
   }
 
-  /// \brief Returns an iterator to the first instruction in this block that is
+  /// Returns an iterator to the first instruction in this block that is
   /// suitable for inserting a non-PHI instruction.
   ///
   /// In particular, it skips all PHIs and LandingPad instructions.
   const_iterator getFirstInsertionPt() const;
   iterator getFirstInsertionPt() {
     return static_cast<const BasicBlock *>(this)
                                           ->getFirstInsertionPt().getNonConst();
   }
 
-  /// \brief Unlink 'this' from the containing function, but do not delete it.
+  /// Unlink 'this' from the containing function, but do not delete it.
   void removeFromParent();
 
-  /// \brief Unlink 'this' from the containing function and delete it.
+  /// Unlink 'this' from the containing function and delete it.
   ///
   // \returns an iterator pointing to the element after the erased one.
   SymbolTableList<BasicBlock>::iterator eraseFromParent();
 
-  /// \brief Unlink this basic block from its current function and insert it
+  /// Unlink this basic block from its current function and insert it
   /// into the function that \p MovePos lives in, right before \p MovePos.
   void moveBefore(BasicBlock *MovePos);
 
-  /// \brief Unlink this basic block from its current function and insert it
+  /// Unlink this basic block from its current function and insert it
   /// right after \p MovePos in the function \p MovePos lives in.
   void moveAfter(BasicBlock *MovePos);
 
-  /// \brief Insert unlinked basic block into a function.
+  /// Insert unlinked basic block into a function.
   ///
   /// Inserts an unlinked basic block into \c Parent.  If \c InsertBefore is
   /// provided, inserts before that basic block, otherwise inserts at the end.
   ///
   /// \pre \a getParent() is \c nullptr.
   void insertInto(Function *Parent, BasicBlock *InsertBefore = nullptr);
 
-  /// \brief Return the predecessor of this block if it has a single predecessor
+  /// Return the predecessor of this block if it has a single predecessor
   /// block. Otherwise return a null pointer.
   const BasicBlock *getSinglePredecessor() const;
   BasicBlock *getSinglePredecessor() {
     return const_cast<BasicBlock *>(
                  static_cast<const BasicBlock *>(this)->getSinglePredecessor());
   }
 
-  /// \brief Return the predecessor of this block if it has a unique predecessor
+  /// Return the predecessor of this block if it has a unique predecessor
   /// block. Otherwise return a null pointer.
   ///
   /// Note that unique predecessor doesn't mean single edge, there can be
   /// multiple edges from the unique predecessor to this block (for example a
   /// switch statement with multiple cases having the same destination).
   const BasicBlock *getUniquePredecessor() const;
   BasicBlock *getUniquePredecessor() {
     return const_cast<BasicBlock *>(
                  static_cast<const BasicBlock *>(this)->getUniquePredecessor());
   }
 
-  /// \brief Return the successor of this block if it has a single successor.
+  /// Return the successor of this block if it has a single successor.
   /// Otherwise return a null pointer.
   ///
   /// This method is analogous to getSinglePredecessor above.
   const BasicBlock *getSingleSuccessor() const;
   BasicBlock *getSingleSuccessor() {
     return const_cast<BasicBlock *>(
                    static_cast<const BasicBlock *>(this)->getSingleSuccessor());
   }
 
-  /// \brief Return the successor of this block if it has a unique successor.
+  /// Return the successor of this block if it has a unique successor.
   /// Otherwise return a null pointer.
   ///
   /// This method is analogous to getUniquePredecessor above.
   const BasicBlock *getUniqueSuccessor() const;
   BasicBlock *getUniqueSuccessor() {
     return const_cast<BasicBlock *>(
                    static_cast<const BasicBlock *>(this)->getUniqueSuccessor());
   }
public:
   /// Returns a range that iterates over the phis in the basic block.
   ///
   /// Note that this cannot be used with basic blocks that have no terminator.
   iterator_range<const_phi_iterator> phis() const {
     return const_cast<BasicBlock *>(this)->phis();
   }
   iterator_range<phi_iterator> phis();
 
-  /// \brief Return the underlying instruction list container.
+  /// Return the underlying instruction list container.
   ///
   /// Currently you need to access the underlying instruction list container
   /// directly if you want to modify it.
   const InstListType &getInstList() const { return InstList; }
         InstListType &getInstList()       { return InstList; }
 
-  /// \brief Returns a pointer to a member of the instruction list.
+  /// Returns a pointer to a member of the instruction list.
   static InstListType BasicBlock::*getSublistAccess(Instruction*) {
     return &BasicBlock::InstList;
   }
 
-  /// \brief Returns a pointer to the symbol table if one exists.
+  /// Returns a pointer to the symbol table if one exists.
   ValueSymbolTable *getValueSymbolTable();
 
-  /// \brief Methods for support type inquiry through isa, cast, and dyn_cast.
+  /// Methods for support type inquiry through isa, cast, and dyn_cast.
   static bool classof(const Value *V) {
     return V->getValueID() == Value::BasicBlockVal;
   }
 
-  /// \brief Cause all subinstructions to "let go" of all the references that
+  /// Cause all subinstructions to "let go" of all the references that
   /// said subinstructions are maintaining.
   ///
   /// This allows one to 'delete' a whole class at a time, even though there may
   /// be circular references... first all references are dropped, and all use
   /// counts go to zero.  Then everything is delete'd for real.  Note that no
   /// operations are valid on an object that has "dropped all references",
   /// except operator delete.
   void dropAllReferences();
 
-  /// \brief Notify the BasicBlock that the predecessor \p Pred is no longer
+  /// Notify the BasicBlock that the predecessor \p Pred is no longer
   /// able to reach it.
   ///
   /// This is actually not used to update the Predecessor list, but is actually
   /// used to update the PHI nodes that reside in the block.  Note that this
   /// should be called while the predecessor still refers to this block.
   void removePredecessor(BasicBlock *Pred, bool DontDeleteUselessPHIs = false);
 
   bool canSplitPredecessors() const;
 
-  /// \brief Split the basic block into two basic blocks at the specified
+  /// Split the basic block into two basic blocks at the specified
   /// instruction.
   ///
   /// Note that all instructions BEFORE the specified iterator stay as part of
   /// the original basic block, an unconditional branch is added to the original
   /// BB, and the rest of the instructions in the BB are moved to the new BB,
   /// including the old terminator.  The newly formed BasicBlock is returned.
   /// This function invalidates the specified iterator.
   ///
   /// Note that this only works on well formed basic blocks (must have a
   /// terminator), and 'I' must not be the end of instruction list (which would
   /// cause a degenerate basic block to be formed, having a terminator inside of
   /// the basic block).
   ///
   /// Also note that this doesn't preserve any passes. To split blocks while
   /// keeping loop information consistent, use the SplitBlock utility function.
   BasicBlock *splitBasicBlock(iterator I, const Twine &BBName = "");
   BasicBlock *splitBasicBlock(Instruction *I, const Twine &BBName = "") {
     return splitBasicBlock(I->getIterator(), BBName);
   }
 
-  /// \brief Returns true if there are any uses of this basic block other than
+  /// Returns true if there are any uses of this basic block other than
   /// direct branches, switches, etc. to it.
   bool hasAddressTaken() const { return getSubclassDataFromValue() != 0; }
 
-  /// \brief Update all phi nodes in this basic block's successors to refer to
+  /// Update all phi nodes in this basic block's successors to refer to
   /// basic block \p New instead of to it.
   void replaceSuccessorsPhiUsesWith(BasicBlock *New);
 
-  /// \brief Return true if this basic block is an exception handling block.
+  /// Return true if this basic block is an exception handling block.
   bool isEHPad() const { return getFirstNonPHI()->isEHPad(); }
 
-  /// \brief Return true if this basic block is a landing pad.
+  /// Return true if this basic block is a landing pad.
   ///
   /// Being a ``landing pad'' means that the basic block is the destination of
   /// the 'unwind' edge of an invoke instruction.
   bool isLandingPad() const;
 
-  /// \brief Return the landingpad instruction associated with the landing pad.
+  /// Return the landingpad instruction associated with the landing pad.
   const LandingPadInst *getLandingPadInst() const;
   LandingPadInst *getLandingPadInst() {
     return const_cast<LandingPadInst *>(
                     static_cast<const BasicBlock *>(this)->getLandingPadInst());
   }
 
-  /// \brief Return true if it is legal to hoist instructions into this block.
+  /// Return true if it is legal to hoist instructions into this block.
   bool isLegalToHoistInto() const;
 
   Optional<uint64_t> getIrrLoopHeaderWeight() const;
 
 private:
-  /// \brief Increment the internal refcount of the number of BlockAddresses
+  /// Increment the internal refcount of the number of BlockAddresses
   /// referencing this BasicBlock by \p Amt.
   ///
   /// This is almost always 0, sometimes one possibly, but almost never 2, and
   /// inconceivably 3 or more.
   void AdjustBlockAddressRefCount(int Amt) {
     setValueSubclassData(getSubclassDataFromValue()+Amt);
     assert((int)(signed char)getSubclassDataFromValue() >= 0 &&
            "Refcount wrap-around");
   }
 
-  /// \brief Shadow Value::setValueSubclassData with a private forwarding method
+  /// Shadow Value::setValueSubclassData with a private forwarding method
   /// so that any future subclasses cannot accidentally use it.
   void setValueSubclassData(unsigned short D) {
     Value::setValueSubclassData(D);
   }
 };
 
 // Create wrappers for C Binding types (see CBindingWrapping.h).
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(BasicBlock, LLVMBasicBlockRef)
 
 } // end namespace llvm
 
 #endif // LLVM_IR_BASICBLOCK_H