aaronpuchert created this revision.
aaronpuchert added a reviewer: gribozavr2.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Instead of checking within the checks whether they apply, we check
before calling them. This allows us to fetch the CommandInfo only once
instead of for every check. Also eliminates some redundant checks and
deduplicates code in Sema::checkBlockCommandDuplicate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113793

Files:
  clang/include/clang/AST/CommentSema.h
  clang/lib/AST/CommentSema.cpp

Index: clang/lib/AST/CommentSema.cpp
===================================================================
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -55,7 +55,9 @@
   BlockCommandComment *BC = new (Allocator) BlockCommandComment(LocBegin, LocEnd,
                                                                 CommandID,
                                                                 CommandMarker);
-  checkContainerDecl(BC);
+  const CommandInfo *Info = Traits.getCommandInfo(CommandID);
+  if (Info->IsRecordLikeDetailCommand)
+    checkContainerDecl(BC);
   return BC;
 }
 
@@ -67,13 +69,20 @@
 void Sema::actOnBlockCommandFinish(BlockCommandComment *Command,
                                    ParagraphComment *Paragraph) {
   Command->setParagraph(Paragraph);
-  checkBlockCommandEmptyParagraph(Command);
-  checkBlockCommandDuplicate(Command);
+  const CommandInfo *Info = Traits.getCommandInfo(Command->getCommandID());
+  if (!Info->IsEmptyParagraphAllowed)
+    checkBlockCommandEmptyParagraph(Command);
+  if (Info->IsBriefCommand)
+    checkBlockCommandDuplicate(Command, &Sema::BriefCommand);
+  else if (Info->IsHeaderfileCommand)
+    checkBlockCommandDuplicate(Command, &Sema::HeaderfileCommand);
   if (ThisDeclInfo) {
     // These checks only make sense if the comment is attached to a
     // declaration.
-    checkReturnsCommand(Command);
-    checkDeprecatedCommand(Command);
+    if (Info->IsReturnsCommand)
+      checkReturnsCommand(Command);
+    if (Info->IsDeprecatedCommand)
+      checkDeprecatedCommand(Command);
   }
 }
 
@@ -96,10 +105,6 @@
 }
 
 void Sema::checkFunctionDeclVerbatimLine(const BlockCommandComment *Comment) {
-  const CommandInfo *Info = Traits.getCommandInfo(Comment->getCommandID());
-  if (!Info->IsFunctionDeclarationCommand)
-    return;
-
   unsigned DiagSelect;
   switch (Comment->getCommandID()) {
     case CommandTraits::KCI_function:
@@ -129,9 +134,6 @@
 }
 
 void Sema::checkContainerDeclVerbatimLine(const BlockCommandComment *Comment) {
-  const CommandInfo *Info = Traits.getCommandInfo(Comment->getCommandID());
-  if (!Info->IsRecordLikeDeclarationCommand)
-    return;
   unsigned DiagSelect;
   switch (Comment->getCommandID()) {
     case CommandTraits::KCI_class:
@@ -168,8 +170,7 @@
 }
 
 void Sema::checkContainerDecl(const BlockCommandComment *Comment) {
-  const CommandInfo *Info = Traits.getCommandInfo(Comment->getCommandID());
-  if (!Info->IsRecordLikeDetailCommand || isRecordLikeDecl())
+  if (isRecordLikeDecl())
     return;
   unsigned DiagSelect;
   switch (Comment->getCommandID()) {
@@ -275,6 +276,7 @@
 void Sema::actOnParamCommandFinish(ParamCommandComment *Command,
                                    ParagraphComment *Paragraph) {
   Command->setParagraph(Paragraph);
+  assert(!Traits.getCommandInfo(Command->getCommandID())->IsEmptyParagraphAllowed);
   checkBlockCommandEmptyParagraph(Command);
 }
 
@@ -358,6 +360,7 @@
 void Sema::actOnTParamCommandFinish(TParamCommandComment *Command,
                                     ParagraphComment *Paragraph) {
   Command->setParagraph(Paragraph);
+  assert(!Traits.getCommandInfo(Command->getCommandID())->IsEmptyParagraphAllowed);
   checkBlockCommandEmptyParagraph(Command);
 }
 
@@ -450,8 +453,12 @@
                               CommandID,
                               TextBegin,
                               Text);
-  checkFunctionDeclVerbatimLine(VL);
-  checkContainerDeclVerbatimLine(VL);
+
+  const CommandInfo *Info = Traits.getCommandInfo(CommandID);
+  if (Info->IsFunctionDeclarationCommand)
+    checkFunctionDeclVerbatimLine(VL);
+  if (Info->IsRecordLikeDeclarationCommand)
+    checkContainerDeclVerbatimLine(VL);
   return VL;
 }
 
@@ -561,9 +568,6 @@
 }
 
 void Sema::checkBlockCommandEmptyParagraph(BlockCommandComment *Command) {
-  if (Traits.getCommandInfo(Command->getCommandID())->IsEmptyParagraphAllowed)
-    return;
-
   ParagraphComment *Paragraph = Command->getParagraph();
   if (Paragraph->isWhitespace()) {
     SourceLocation DiagLoc;
@@ -579,9 +583,6 @@
 }
 
 void Sema::checkReturnsCommand(const BlockCommandComment *Command) {
-  if (!Traits.getCommandInfo(Command->getCommandID())->IsReturnsCommand)
-    return;
-
   assert(ThisDeclInfo && "should not call this check on a bare comment");
 
   // We allow the return command for all @properties because it can be used
@@ -624,25 +625,14 @@
     << Command->getSourceRange();
 }
 
-void Sema::checkBlockCommandDuplicate(const BlockCommandComment *Command) {
-  const CommandInfo *Info = Traits.getCommandInfo(Command->getCommandID());
+void Sema::checkBlockCommandDuplicate(const BlockCommandComment *Command, const BlockCommandComment *Sema::*LastOccurrence) {
   const BlockCommandComment *PrevCommand = nullptr;
-  if (Info->IsBriefCommand) {
-    if (!BriefCommand) {
-      BriefCommand = Command;
-      return;
-    }
-    PrevCommand = BriefCommand;
-  } else if (Info->IsHeaderfileCommand) {
-    if (!HeaderfileCommand) {
-      HeaderfileCommand = Command;
-      return;
-    }
-    PrevCommand = HeaderfileCommand;
-  } else {
-    // We don't want to check this command for duplicates.
+  if (!(this->*LastOccurrence)) {
+    this->*LastOccurrence = Command;
     return;
   }
+  PrevCommand = this->*LastOccurrence;
+
   StringRef CommandName = Command->getCommandName(Traits);
   StringRef PrevCommandName = PrevCommand->getCommandName(Traits);
   Diag(Command->getLocation(), diag::warn_doc_block_command_duplicate)
@@ -663,9 +653,6 @@
 }
 
 void Sema::checkDeprecatedCommand(const BlockCommandComment *Command) {
-  if (!Traits.getCommandInfo(Command->getCommandID())->IsDeprecatedCommand)
-    return;
-
   assert(ThisDeclInfo && "should not call this check on a bare comment");
 
   const Decl *D = ThisDeclInfo->CommentDecl;
Index: clang/include/clang/AST/CommentSema.h
===================================================================
--- clang/include/clang/AST/CommentSema.h
+++ clang/include/clang/AST/CommentSema.h
@@ -188,7 +188,7 @@
 
   /// Emit diagnostics about duplicate block commands that should be
   /// used only once per comment, e.g., \and \\returns.
-  void checkBlockCommandDuplicate(const BlockCommandComment *Command);
+  void checkBlockCommandDuplicate(const BlockCommandComment *Command, const BlockCommandComment *Sema::*LastOccurrence);
 
   void checkDeprecatedCommand(const BlockCommandComment *Comment);
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to