[clang] 8bb54da - Allow getRawCommentForDecl to find comments in macros

Dmitri Gribenko via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 26 01:13:04 PST 2023


Author: Dana Jansens
Date: 2023-01-26T10:12:57+01:00
New Revision: 8bb54da5da3194b71b54f70c6cc55485cf2623b0

URL: https://github.com/llvm/llvm-project/commit/8bb54da5da3194b71b54f70c6cc55485cf2623b0
DIFF: https://github.com/llvm/llvm-project/commit/8bb54da5da3194b71b54f70c6cc55485cf2623b0.diff

LOG: Allow getRawCommentForDecl to find comments in macros

The key part of getRawCommentForDecl() required to find a comment
is determining where to look for it. The location of the decl
itself is usually right, except when macros get involved. The
comment in the macro is stored in RawCommentList at the spelling
location of the decl, not at the place where the decl comes into
being as the macro is instantiated.

getDeclLocForCommentSearch() already contained to branches to try
handle comments inside macros, and we are able to replace them
and handle more cases as well, by returning the spelling location
of the decl's begin location. That is:
  SourceMgr.getSpellingLoc(D->getBeginLoc())

Reviewed By: gribozavr2

Differential Revision: https://reviews.llvm.org/D142560

Added: 
    

Modified: 
    clang/lib/AST/ASTContext.cpp
    clang/test/Index/annotate-comments-objc.m

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 3ba8b90898b30..18a041d7a72ab 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -181,22 +181,96 @@ static SourceLocation getDeclLocForCommentSearch(const Decl *D,
 
   const SourceLocation DeclLoc = D->getLocation();
   if (DeclLoc.isMacroID()) {
-    if (isa<TypedefDecl>(D)) {
-      // If location of the typedef name is in a macro, it is because being
-      // declared via a macro. Try using declaration's starting location as
-      // the "declaration location".
-      return D->getBeginLoc();
-    }
-
-    if (const auto *TD = dyn_cast<TagDecl>(D)) {
-      // If location of the tag decl is inside a macro, but the spelling of
-      // the tag name comes from a macro argument, it looks like a special
-      // macro like NS_ENUM is being used to define the tag decl.  In that
-      // case, adjust the source location to the expansion loc so that we can
-      // attach the comment to the tag decl.
-      if (SourceMgr.isMacroArgExpansion(DeclLoc) && TD->isCompleteDefinition())
-        return SourceMgr.getExpansionLoc(DeclLoc);
+    // There are (at least) three types of macros we care about here.
+    //
+    // 1. Macros that are used in the definition of a type outside the macro,
+    //    with a comment attached at the macro call site.
+    //    ```
+    //    #define MAKE_NAME(Foo) Name##Foo
+    //
+    //    /// Comment is here, where we use the macro.
+    //    struct MAKE_NAME(Foo) {
+    //        int a;
+    //        int b;
+    //    };
+    //    ```
+    // 2. Macros that define whole things along with the comment.
+    //    ```
+    //    #define MAKE_METHOD(name) \
+    //      /** Comment is here, inside the macro. */ \
+    //      void name() {}
+    //
+    //    struct S {
+    //      MAKE_METHOD(f)
+    //    }
+    //    ```
+    // 3. Macros that both declare a type and name a decl outside the macro.
+    //    ```
+    //    /// Comment is here, where we use the macro.
+    //    typedef NS_ENUM(NSInteger, Size) {
+    //        SizeWidth,
+    //        SizeHeight
+    //    };
+    //    ```
+    //    In this case NS_ENUM declares am enum type, and uses the same name for
+    //    the typedef declaration that appears outside the macro. The comment
+    //    here should be applied to both declarations inside and outside the
+    //    macro.
+    //
+    // We have found a Decl name that comes from inside a macro, but
+    // Decl::getLocation() returns the place where the macro is being called.
+    // If the declaration (and not just the name) resides inside the macro,
+    // then we want to map Decl::getLocation() into the macro to where the
+    // declaration and its attached comment (if any) were written.
+    //
+    // This mapping into the macro is done by mapping the location to its
+    // spelling location, however even if the declaration is inside a macro,
+    // the name's spelling can come from a macro argument (case 2 above). In
+    // this case mapping the location to the spelling location finds the
+    // argument's position (at `f` in MAKE_METHOD(`f`) above), which is not
+    // where the declaration and its comment are located.
+    //
+    // To avoid this issue, we make use of Decl::getBeginLocation() instead.
+    // While the declaration's position is where the name is written, the
+    // comment is always attached to the begining of the declaration, not to
+    // the name.
+    //
+    // In the first case, the begin location of the decl is outside the macro,
+    // at the location of `typedef`. This is where the comment is found as
+    // well. The begin location is not inside a macro, so it's spelling
+    // location is the same.
+    //
+    // In the second case, the begin location of the decl is the call to the
+    // macro, at `MAKE_METHOD`. However its spelling location is inside the
+    // the macro at the location of `void`. This is where the comment is found
+    // again.
+    //
+    // In the third case, there's no correct single behaviour. We want to use
+    // the comment outside the macro for the definition that's inside the macro.
+    // There is also a definition outside the macro, and we want the comment to
+    // apply to both. The cases we care about here is NS_ENUM() and
+    // NS_OPTIONS(). In general, if an enum is defined inside a macro, we should
+    // try to find the comment there.
+
+    // This is handling case 3 for NS_ENUM() and NS_OPTIONS(), which define
+    // enum types inside the macro.
+    if (isa<EnumDecl>(D)) {
+      SourceLocation MacroCallLoc = SourceMgr.getExpansionLoc(DeclLoc);
+      if (auto BufferRef =
+              SourceMgr.getBufferOrNone(SourceMgr.getFileID(MacroCallLoc));
+          BufferRef.has_value()) {
+        llvm::StringRef buffer = BufferRef->getBuffer().substr(
+            SourceMgr.getFileOffset(MacroCallLoc));
+        if (buffer.starts_with("NS_ENUM(") ||
+            buffer.starts_with("NS_OPTIONS(")) {
+          // We want to use the comment on the call to NS_ENUM and NS_OPTIONS
+          // macros for the types defined inside the macros, which is at the
+          // expansion location.
+          return MacroCallLoc;
+        }
+      }
     }
+    return SourceMgr.getSpellingLoc(D->getBeginLoc());
   }
 
   return DeclLoc;

diff  --git a/clang/test/Index/annotate-comments-objc.m b/clang/test/Index/annotate-comments-objc.m
index b4671519c4d91..41ede39baa870 100644
--- a/clang/test/Index/annotate-comments-objc.m
+++ b/clang/test/Index/annotate-comments-objc.m
@@ -48,17 +48,31 @@ - (void)method1_isdoxy4; /*!< method1_isdoxy4 IS_DOXYGEN_SINGLE */
 // attach unrelated comments in the following cases where tag decls are
 // embedded in declarators.
 
-#define DECLARE_FUNCTION() \
+#define DECLARE_FUNCTIONS(suffix) \
+    /** functionFromMacro IS_DOXYGEN_SINGLE */ \
     void functionFromMacro(void) { \
       typedef struct Struct_notdoxy Struct_notdoxy; \
+    } \
+    /** functionFromMacroWithSuffix IS_DOXYGEN_SINGLE */ \
+    void functionFromMacro##suffix(void) { \
+      typedef struct Struct_notdoxy Struct_notdoxy; \
     }
 
 /// IS_DOXYGEN_NOT_ATTACHED
-DECLARE_FUNCTION()
+DECLARE_FUNCTIONS(WithSuffix)
 
 /// typedef_isdoxy1 IS_DOXYGEN_SINGLE
 typedef struct Struct_notdoxy *typedef_isdoxy1;
 
+#define DECLARE_ENUMS(name) \
+  /** enumFromMacro IS_DOXYGEN_SINGLE */ \
+  enum enumFromMacro { A }; \
+  /** namedEnumFromMacro IS_DOXYGEN_SINGLE */ \
+  enum name { B };
+
+/// IS_DOXYGEN_NOT_ATTACHED
+DECLARE_ENUMS(namedEnumFromMacro)
+
 #endif
 
 // RUN: rm -rf %t
@@ -121,5 +135,8 @@ void functionFromMacro(void) { \
 // CHECK: annotate-comments-objc.m:43:22: EnumDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE
 // CHECK: annotate-comments-objc.m:43:22: TypedefDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE
 // CHECK: annotate-comments-objc.m:43:22: EnumDecl=An_NS_ENUM_isdoxy1:{{.*}} An_NS_ENUM_isdoxy1 IS_DOXYGEN_SINGLE
-// CHECK: annotate-comments-objc.m:60:32: TypedefDecl=typedef_isdoxy1:{{.*}} typedef_isdoxy1 IS_DOXYGEN_SINGLE
-
+// CHECK: annotate-comments-objc.m:62:1: FunctionDecl=functionFromMacro:{{.*}} BriefComment=[functionFromMacro IS_DOXYGEN_SINGLE]
+// CHECK: annotate-comments-objc.m:62:1: FunctionDecl=functionFromMacroWithSuffix:{{.*}} BriefComment=[functionFromMacroWithSuffix IS_DOXYGEN_SINGLE]
+// CHECK: annotate-comments-objc.m:65:32: TypedefDecl=typedef_isdoxy1:{{.*}} typedef_isdoxy1 IS_DOXYGEN_SINGLE
+// CHECK: annotate-comments-objc.m:74:1: EnumDecl=enumFromMacro:{{.*}} BriefComment=[enumFromMacro IS_DOXYGEN_SINGLE]
+// CHECK: annotate-comments-objc.m:74:15: EnumDecl=namedEnumFromMacro:{{.*}} BriefComment=[namedEnumFromMacro IS_DOXYGEN_SINGLE]


        


More information about the cfe-commits mailing list