Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC][analyzer][docs] Migrate 'annotations.html' to RST #122246

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

NagyDonat
Copy link
Contributor

This commit migrates the contents of 'annotations.html' in the old HTML-based documentation of the Clang static analyzer to the new RST-based documentation.

During this conversion I reordered the sections of this documentation file by placing the section "Custom Assertion Handlers" as a subsection of "Annotations to Enhance Generic Checks". (The primary motivation was that Sphinx complained about inconsistent section levels; with this change I preserved that sections describing individual annotations are all on the same level.)

Apart from this change and the format conversion, I didn't review, validate or edit the contents of this documentation file because I think it would be better to place any additional changes in separate commits.

This commit migrates the contents of 'annotations.html' in the old
HTML-based documentation of the Clang static analyzer to the new
RST-based documentation.

During this conversion I reordered the sections of this documentation
file by placing the section "Custom Assertion Handlers" as a subsection
of "Annotations to Enhance Generic Checks". (The primary motivation was
that Sphinx complained about inconsistent section levels; with this
change I preserved that sections describing individual annotations are
all on the same level.)

Apart from this change and the format conversion, I didn't review,
validate or edit the contents of this documentation file because I think
it would be better to place any additional changes in separate commits.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:static analyzer labels Jan 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 9, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Donát Nagy (NagyDonat)

Changes

This commit migrates the contents of 'annotations.html' in the old HTML-based documentation of the Clang static analyzer to the new RST-based documentation.

During this conversion I reordered the sections of this documentation file by placing the section "Custom Assertion Handlers" as a subsection of "Annotations to Enhance Generic Checks". (The primary motivation was that Sphinx complained about inconsistent section levels; with this change I preserved that sections describing individual annotations are all on the same level.)

Apart from this change and the format conversion, I didn't review, validate or edit the contents of this documentation file because I think it would be better to place any additional changes in separate commits.


Patch is 59.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/122246.diff

7 Files Affected:

  • (renamed) clang/docs/analyzer/images/example_attribute_nonnull.png ()
  • (renamed) clang/docs/analyzer/images/example_cf_returns_retained.png ()
  • (renamed) clang/docs/analyzer/images/example_ns_returns_retained.png ()
  • (modified) clang/docs/analyzer/user-docs.rst (+1)
  • (added) clang/docs/analyzer/user-docs/Annotations.rst (+685)
  • (modified) clang/include/clang/Basic/AttrDocs.td (+1-1)
  • (modified) clang/www/analyzer/annotations.html (+7-759)
diff --git a/clang/www/analyzer/images/example_attribute_nonnull.png b/clang/docs/analyzer/images/example_attribute_nonnull.png
similarity index 100%
rename from clang/www/analyzer/images/example_attribute_nonnull.png
rename to clang/docs/analyzer/images/example_attribute_nonnull.png
diff --git a/clang/www/analyzer/images/example_cf_returns_retained.png b/clang/docs/analyzer/images/example_cf_returns_retained.png
similarity index 100%
rename from clang/www/analyzer/images/example_cf_returns_retained.png
rename to clang/docs/analyzer/images/example_cf_returns_retained.png
diff --git a/clang/www/analyzer/images/example_ns_returns_retained.png b/clang/docs/analyzer/images/example_ns_returns_retained.png
similarity index 100%
rename from clang/www/analyzer/images/example_ns_returns_retained.png
rename to clang/docs/analyzer/images/example_ns_returns_retained.png
diff --git a/clang/docs/analyzer/user-docs.rst b/clang/docs/analyzer/user-docs.rst
index dd53ae143148c0..e265f033a2c540 100644
--- a/clang/docs/analyzer/user-docs.rst
+++ b/clang/docs/analyzer/user-docs.rst
@@ -12,4 +12,5 @@ Contents:
    user-docs/FilingBugs
    user-docs/CrossTranslationUnit
    user-docs/TaintAnalysisConfiguration
+   user-docs/Annotations
    user-docs/FAQ
diff --git a/clang/docs/analyzer/user-docs/Annotations.rst b/clang/docs/analyzer/user-docs/Annotations.rst
new file mode 100644
index 00000000000000..778047b82d517d
--- /dev/null
+++ b/clang/docs/analyzer/user-docs/Annotations.rst
@@ -0,0 +1,685 @@
+==================
+Source Annotations
+==================
+
+The Clang frontend supports several source-level annotations in the form of
+`GCC-style attributes <https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html>`_
+and pragmas that can help make using the Clang Static Analyzer more useful.
+These annotations can both help suppress false positives as well as enhance the
+analyzer's ability to find bugs.
+
+This page gives a practical overview of such annotations. For more technical
+specifics regarding Clang-specific annotations please see the Clang's list of
+`language extensions <https://clang.llvm.org/docs/LanguageExtensions.html>`_.
+Details of "standard" GCC attributes (that Clang also supports) can
+be found in the `GCC manual <https://gcc.gnu.org/onlinedocs/gcc/>`_, with the
+majority of the relevant attributes being in the section on
+`function attributes <https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html>`_.
+
+Note that attributes that are labeled **Clang-specific** are not
+recognized by GCC. Their use can be conditioned using preprocessor macros
+(examples included on this page).
+
+.. contents::
+   :local:
+
+Annotations to Enhance Generic Checks
+_____________________________________
+
+Null Pointer Checking
+#####################
+
+Attribute 'nonnull'
+-------------------
+
+The analyzer recognizes the GCC attribute 'nonnull', which indicates that a
+function expects that a given function parameter is not a null pointer.
+Specific details of the syntax of using the 'nonnull' attribute can be found in
+`GCC's documentation <https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-nonnull-function-attribute>`_.
+
+Both the Clang compiler and GCC will flag warnings for simple cases where a
+null pointer is directly being passed to a function with a 'nonnull' parameter
+(e.g., as a constant). The analyzer extends this checking by using its deeper
+symbolic analysis to track what pointer values are potentially null and then
+flag warnings when they are passed in a function call via a 'nonnull'
+parameter.
+
+**Example**
+
+.. code-block:: c
+
+  int bar(int*p, int q, int *r) __attribute__((nonnull(1,3)));
+
+  int foo(int *p, int *q) {
+     return !p ? bar(q, 2, p)
+               : bar(p, 2, q);
+  }
+
+Running ``scan-build`` over this source produces the following output:
+
+.. image:: ../images/example_attribute_nonnull.png
+
+Custom Assertion Handlers
+#########################
+
+The analyzer exploits code assertions by pruning off paths where the
+assertion condition is false. The idea is capture any program invariants
+specified in the assertion that the developer may know but is not immediately
+apparent in the code itself. In this way assertions make implicit assumptions
+explicit in the code, which not only makes the analyzer more accurate when
+finding bugs, but can help others better able to understand your code as well.
+It can also help remove certain kinds of analyzer false positives by pruning off
+false paths.
+
+In order to exploit assertions, however, the analyzer must understand when it
+encounters an "assertion handler". Typically assertions are
+implemented with a macro, with the macro performing a check for the assertion
+condition and, when the check fails, calling an assertion handler.  For
+example, consider the following code fragment:
+
+.. code-block: c
+
+  void foo(int *p) {
+    assert(p != NULL);
+  }
+
+When this code is preprocessed on Mac OS X it expands to the following:
+
+.. code-block: c
+
+  void foo(int *p) {
+    (__builtin_expect(!(p != NULL), 0) ? __assert_rtn(__func__, "t.c", 4, "p != NULL") : (void)0);
+  }
+
+In this example, the assertion handler is ``__assert_rtn``. When called,
+most assertion handlers typically print an error and terminate the program. The
+analyzer can exploit such semantics by ending the analysis of a path once it
+hits a call to an assertion handler.
+
+The trick, however, is that the analyzer needs to know that a called function
+is an assertion handler; otherwise the analyzer might assume the function call
+returns and it will continue analyzing the path where the assertion condition
+failed. This can lead to false positives, as the assertion condition usually
+implies a safety condition (e.g., a pointer is not null) prior to performing
+some action that depends on that condition (e.g., dereferencing a pointer).
+
+The analyzer knows about several well-known assertion handlers, but can
+automatically infer if a function should be treated as an assertion handler if
+it is annotated with the 'noreturn' attribute or the (Clang-specific)
+'analyzer_noreturn' attribute. Note that, currently, clang does not support
+these attributes on Objective-C methods and C++ methods.
+
+Attribute 'noreturn'
+--------------------
+
+The 'noreturn' attribute is a GCC attribute that can be placed on the
+declarations of functions. It means exactly what its name implies: a function
+with a 'noreturn' attribute should never return.
+
+Specific details of the syntax of using the 'noreturn' attribute can be found
+in `GCC's documentation <https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute>`__.
+
+Not only does the analyzer exploit this information when pruning false paths,
+but the compiler also takes it seriously and will generate different code (and
+possibly better optimized) under the assumption that the function does not
+return.
+
+**Example**
+
+On Mac OS X, the function prototype for ``__assert_rtn`` (declared in
+``assert.h``) is specifically annotated with the 'noreturn' attribute:
+
+.. code-block: c
+
+  void __assert_rtn(const char *, const char *, int, const char *) __attribute__((__noreturn__));
+
+Attribute 'analyzer_noreturn' (Clang-specific)
+----------------------------------------------
+
+The Clang-specific 'analyzer_noreturn' attribute is almost identical to
+'noreturn' except that it is ignored by the compiler for the purposes of code
+generation.
+
+This attribute is useful for annotating assertion handlers that actually
+*can* return, but for the purpose of using the analyzer we want to
+pretend that such functions do not return.
+
+Because this attribute is Clang-specific, its use should be conditioned with
+the use of preprocessor macros.
+
+**Example**
+
+.. code-block: c
+
+  #ifndef CLANG_ANALYZER_NORETURN
+  #if __has_feature(attribute_analyzer_noreturn)
+  #define CLANG_ANALYZER_NORETURN __attribute__((analyzer_noreturn))
+  #else
+  #define CLANG_ANALYZER_NORETURN
+  #endif
+  #endif
+
+  void my_assert_rtn(const char *, const char *, int, const char *) CLANG_ANALYZER_NORETURN;
+
+Mac OS X API Annotations
+________________________
+
+Cocoa & Core Foundation Memory Management Annotations
+#####################################################
+
+The analyzer supports the proper management of retain counts for
+both Cocoa and Core Foundation objects. This checking is largely based on
+enforcing Cocoa and Core Foundation naming conventions for Objective-C methods
+(Cocoa) and C functions (Core Foundation). Not strictly following these
+conventions can cause the analyzer to miss bugs or flag false positives.
+
+One can educate the analyzer (and others who read your code) about methods or
+functions that deviate from the Cocoa and Core Foundation conventions using the
+attributes described here. However, you should consider using proper naming
+conventions or the `objc_method_family <https://clang.llvm.org/docs/LanguageExtensions.html#the-objc-method-family-attribute>`_
+attribute, if applicable.
+
+.. _ns_returns_retained:
+
+Attribute 'ns_returns_retained' (Clang-specific)
+------------------------------------------------
+
+The GCC-style (Clang-specific) attribute 'ns_returns_retained' allows one to
+annotate an Objective-C method or C function as returning a retained Cocoa
+object that the caller is responsible for releasing (via sending a
+``release`` message to the object). The Foundation framework defines a
+macro ``NS_RETURNS_RETAINED`` that is functionally equivalent to the
+one shown below.
+
+**Placing on Objective-C methods**: For Objective-C methods, this
+annotation essentially tells the analyzer to treat the method as if its name
+begins with "alloc" or "new" or contains the word
+"copy".
+
+**Placing on C functions**: For C functions returning Cocoa objects, the
+analyzer typically does not make any assumptions about whether or not the object
+is returned retained. Explicitly adding the 'ns_returns_retained' attribute to C
+functions allows the analyzer to perform extra checking.
+
+**Example**
+
+.. code-block: objc
+
+  #import <Foundation/Foundation.h>;
+
+  #ifndef __has_feature      // Optional.
+  #define __has_feature(x) 0 // Compatibility with non-clang compilers.
+  #endif
+
+  #ifndef NS_RETURNS_RETAINED
+  #if __has_feature(attribute_ns_returns_retained)
+  #define NS_RETURNS_RETAINED __attribute__((ns_returns_retained))
+  #else
+  #define NS_RETURNS_RETAINED
+  #endif
+  #endif
+
+  @interface MyClass : NSObject {}
+  - (NSString*) returnsRetained NS_RETURNS_RETAINED;
+  - (NSString*) alsoReturnsRetained;
+  @end
+
+  @implementation MyClass
+  - (NSString*) returnsRetained {
+    return [[NSString alloc] initWithCString:"no leak here"];
+  }
+  - (NSString*) alsoReturnsRetained {
+    return [[NSString alloc] initWithCString:"flag a leak"];
+  }
+  @end
+
+Running ``scan-build`` on this source file produces the following output:
+
+.. image:: ../images/example_ns_returns_retained.png
+
+.. _ns_returns_not_retained:
+
+Attribute 'ns_returns_not_retained' (Clang-specific)
+----------------------------------------------------
+
+The 'ns_returns_not_retained' attribute is the complement of
+'`ns_returns_retained`_'. Where a function or method may appear to obey the
+Cocoa conventions and return a retained Cocoa object, this attribute can be
+used to indicate that the object reference returned should not be considered as
+an "owning" reference being returned to the caller. The Foundation
+framework defines a macro ``NS_RETURNS_NOT_RETAINED`` that is functionally
+equivalent to the one shown below.
+
+Usage is identical to `ns_returns_retained`_.  When using the
+attribute, be sure to declare it within the proper macro that checks for
+its availability, as it is not available in earlier versions of the analyzer:
+
+.. code-block:objc
+
+  #ifndef __has_feature      // Optional.
+  #define __has_feature(x) 0 // Compatibility with non-clang compilers.
+  #endif
+
+  #ifndef NS_RETURNS_NOT_RETAINED
+  #if __has_feature(attribute_ns_returns_not_retained)
+  #define NS_RETURNS_NOT_RETAINED __attribute__((ns_returns_not_retained))
+  #else
+  #define NS_RETURNS_NOT_RETAINED
+  #endif
+  #endif
+
+.. _cf_returns_retained:
+
+Attribute 'cf_returns_retained' (Clang-specific)
+------------------------------------------------
+
+The GCC-style (Clang-specific) attribute 'cf_returns_retained' allows one to
+annotate an Objective-C method or C function as returning a retained Core
+Foundation object that the caller is responsible for releasing. The
+CoreFoundation framework defines a macro ``CF_RETURNS_RETAINED`` that is
+functionally equivalent to the one shown below.
+
+**Placing on Objective-C methods**: With respect to Objective-C methods.,
+this attribute is identical in its behavior and usage to 'ns_returns_retained'
+except for the distinction of returning a Core Foundation object instead of a
+Cocoa object.
+
+This distinction is important for the following reason: as Core Foundation is a
+C API, the analyzer cannot always tell that a pointer return value refers to a
+Core Foundation object. In contrast, it is trivial for the analyzer to
+recognize if a pointer refers to a Cocoa object (given the Objective-C type
+system).
+
+**Placing on C functions**: When placing the attribute
+'cf_returns_retained' on the declarations of C functions, the analyzer
+interprets the function as:
+
+1. Returning a Core Foundation Object
+2. Treating the function as if it its name contained the keywords
+   "create" or "copy". This means the returned object as a
+   +1 retain count that must be released by the caller, either by sending a
+   ``release`` message (via toll-free bridging to an Objective-C object
+   pointer), or calling ``CFRelease`` or a similar function.
+
+**Example**
+
+.. code-block:objc
+
+  #import <Cocoa/Cocoa.h>
+
+  #ifndef __has_feature      // Optional.
+  #define __has_feature(x) 0 // Compatibility with non-clang compilers.
+  #endif
+
+  #ifndef CF_RETURNS_RETAINED
+  #if __has_feature(attribute_cf_returns_retained)
+  #define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+  #else
+  #define CF_RETURNS_RETAINED
+  #endif
+  #endif
+
+  @interface MyClass : NSObject {}
+  - (NSDate*) returnsCFRetained CF_RETURNS_RETAINED;
+  - (NSDate*) alsoReturnsRetained;
+  - (NSDate*) returnsNSRetained NS_RETURNS_RETAINED;
+  @end
+
+  CF_RETURNS_RETAINED
+  CFDateRef returnsRetainedCFDate()  {
+    return CFDateCreate(0, CFAbsoluteTimeGetCurrent());
+  }
+
+  @implementation MyClass
+  - (NSDate*) returnsCFRetained {
+    return (NSDate*) returnsRetainedCFDate(); // No leak.
+  }
+
+  - (NSDate*) alsoReturnsRetained {
+    return (NSDate*) returnsRetainedCFDate(); // Always report a leak.
+  }
+
+  - (NSDate*) returnsNSRetained {
+    return (NSDate*) returnsRetainedCFDate(); // Report a leak when using GC.
+  }
+  @end
+
+Running ``scan-build`` on this example produces the following output:
+
+.. image:: ../images/example_cf_returns_retained.png
+
+Attribute 'cf_returns_not_retained' (Clang-specific)
+----------------------------------------------------
+
+The 'cf_returns_not_retained' attribute is the complement of
+'`cf_returns_retained`_'. Where a function or method may appear to obey the
+Core Foundation or Cocoa conventions and return a retained Core Foundation
+object, this attribute can be used to indicate that the object reference
+returned should not be considered as an "owning" reference being
+returned to the caller. The CoreFoundation framework defines a macro
+**``CF_RETURNS_NOT_RETAINED``** that is functionally equivalent to the one
+shown below.
+
+Usage is identical to cf_returns_retained_. When using the attribute, be sure
+to declare it within the proper macro that checks for its availability, as it
+is not available in earlier versions of the analyzer:
+
+.. code-block:objc
+
+  #ifndef __has_feature      // Optional.
+  #define __has_feature(x) 0 // Compatibility with non-clang compilers.
+  #endif
+
+  #ifndef CF_RETURNS_NOT_RETAINED
+  #if __has_feature(attribute_cf_returns_not_retained)
+  #define CF_RETURNS_NOT_RETAINED __attribute__((cf_returns_not_retained))
+  #else
+  #define CF_RETURNS_NOT_RETAINED
+  #endif
+  #endif
+
+.. _ns_consumed:
+
+Attribute 'ns_consumed' (Clang-specific)
+----------------------------------------
+
+The 'ns_consumed' attribute can be placed on a specific parameter in either
+the declaration of a function or an Objective-C method. It indicates to the
+static analyzer that a ``release`` message is implicitly sent to the
+parameter upon completion of the call to the given function or method. The
+Foundation framework defines a macro ``NS_RELEASES_ARGUMENT`` that
+is functionally equivalent to the ``NS_CONSUMED`` macro shown below.
+
+**Example**
+
+.. code-block:objc
+
+  #ifndef __has_feature      // Optional.
+  #define __has_feature(x) 0 // Compatibility with non-clang compilers.
+  #endif
+
+  #ifndef NS_CONSUMED
+  #if __has_feature(attribute_ns_consumed)
+  #define NS_CONSUMED __attribute__((ns_consumed))
+  #else
+  #define NS_CONSUMED
+  #endif
+  #endif
+
+  void consume_ns(id NS_CONSUMED x);
+
+  void test() {
+    id x = [[NSObject alloc] init];
+    consume_ns(x); // No leak!
+  }
+
+  @interface Foo : NSObject
+  + (void) releaseArg:(id) NS_CONSUMED x;
+  + (void) releaseSecondArg:(id)x second:(id) NS_CONSUMED y;
+  @end
+
+  void test_method() {
+    id x = [[NSObject alloc] init];
+    [Foo releaseArg:x]; // No leak!
+  }
+
+  void test_method2() {
+    id a = [[NSObject alloc] init];
+    id b = [[NSObject alloc] init];
+    [Foo releaseSecondArg:a second:b]; // 'a' is leaked, but 'b' is released.
+  }
+
+Attribute 'cf_consumed' (Clang-specific)
+----------------------------------------
+
+The 'cf_consumed' attribute is practically identical to ns_consumed_. The
+attribute can be placed on a specific parameter in either the declaration of a
+function or an Objective-C method. It indicates to the static analyzer that the
+object reference is implicitly passed to a call to ``CFRelease`` upon
+completion of the call to the given function or method. The CoreFoundation
+framework defines a macro ``CF_RELEASES_ARGUMENT`` that is functionally
+equivalent to the ``CF_CONSUMED`` macro shown below.
+
+Operationally this attribute is nearly identical to 'ns_consumed'.
+
+**Example**
+
+.. code-block:objc
+
+  #ifndef __has_feature      // Optional.
+  #define __has_feature(x) 0 // Compatibility with non-clang compilers.
+  #endif
+
+  #ifndef CF_CONSUMED
+  #if __has_feature(attribute_cf_consumed)
+  #define CF_CONSUMED __attribute__((cf_consumed))
+  #else
+  #define CF_CONSUMED
+  #endif
+  #endif
+
+  void consume_cf(id CF_CONSUMED x);
+  void consume_CFDate(CFDateRef CF_CONSUMED x);
+
+  void test() {
+    id x = [[NSObject alloc] init];
+    consume_cf(x); // No leak!
+  }
+
+  void test2() {
+    CFDateRef date = CFDateCreate(0, CFAbsoluteTimeGetCurrent());
+    consume_CFDate(date); // No leak, including under GC!
+
+  }
+
+  @interface Foo : NSObject
+  + (void) releaseArg:(CFDateRef) CF_CONSUMED x;
+  @end
+
+  void test_method() {
+    CFDateRef date = CFDateCreate(0, CFAbsoluteTimeGetCurrent());
+    [Foo releaseArg:date]; // No leak!
+  }
+
+.. _ns_consumes_self:
+
+Attribute 'ns_consumes_self' (Clang-specific)
+---------------------------------------------
+
+The 'ns_consumes_self' attribute can be placed only on an Objective-C method
+declaration. It indicates that the receiver of the message is
+"consumed" (a single reference count decremented) after the message
+is sent. This matches the semantics of all "init" methods.
+
+One use of this attribute is declare your own init-like methods that do not
+follow the standard Cocoa naming conventions.
+
+**Example**
+
+.. code-block:objc
+  #ifndef __has_feature
+  #define __has_feature(x) 0 // Compatibility with non-clang compilers.
+  #endif
+
+  #ifndef NS_CONSUMES_SELF
+  #if __has_feature((attribute_ns_consumes_self))
+  #define NS_CONSUMES_SELF __attribute__((ns_consumes_self))
+  #else
+  #define NS_CONSUMES_SELF
+  #endif
+  #endif
+
+  @interface MyClass : NSObject
+  - initWith:(MyClass *)x;
+  - nonstandardInitWith:(MyClass *)x NS_CONSUMES_SELF NS_RETURNS_RETAINED;
+  @end
+
+In this example, ``-nonstandardInitWith:`` has the same ownership
+semantics as the init method ``-initWith:``. The sta...
[truncated]

Fix the links that were pointing to the old HTML version of the
annotations documentation.

As a minor unrelated change, also fix a link that tried (and failed) to
point to the static analyzer FAQ from `UsersManual.rst`.

Note that Sphinx converts link target labels like `.. _some_name:` into
a HTML `id` attribute like `some-name` (i.e. underscores are converted
to hyphens).
Copy link
Contributor

@Szelethus Szelethus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we leave a stub in the old HTML page that the docs were moved? Or we haven't done that in the past either?

I understand (and strongly agree with) you not making any meaningful changes to the docs page you moved. With that said, later down the line, we should make sure that checkers are called "checkers", and not "checks" (the latter is used in clang-tidy), and that ownership_.* attributes must be mentioned here as well.

Otherwise LGTM, thanks for the work!

@NagyDonat
Copy link
Contributor Author

Shouldn't we leave a stub in the old HTML page that the docs were moved? Or we haven't done that in the past either?

This commit does leave a stub HTML page (which is closely modeled after the earlier similar stub pages).

I understand (and strongly agree with) you not making any meaningful changes to the docs page you moved. With that said, later down the line, we should make sure that checkers are called "checkers", and not "checks" (the latter is used in clang-tidy), and that ownership_.* attributes must be mentioned here as well.

OK, I'll create a follow-up commit for these. Where should we primarily document the ownership_.* attributes: here or in the global documentation of attributes? (We must avoid duplicating the same content twice, so one location should have the full documentation (which you've written recently) while the other should contain a very brief description (1-2 sentences) and a link to the full docs.

Otherwise LGTM, thanks for the work!

I'm merging this commit now; if anybody else has any suggestions, we can discuss them in follow-up PRs.

@NagyDonat NagyDonat merged commit 21e58ee into llvm:main Jan 13, 2025
9 checks passed
@Szelethus
Copy link
Contributor

Where should we primarily document the ownership_.* attributes: here or in the global documentation of attributes?

I think we should have a section, and a single sentence pointing to the main attribution docs.

@NagyDonat
Copy link
Contributor Author

NagyDonat commented Jan 13, 2025

Buildbot failure https://lab.llvm.org/buildbot/#/builders/190/builds/12715 is completely unrelated to this commit, it's caused by "IO failure on output stream: No space left on device" (on a random Mach-O testcase) 😅

NagyDonat added a commit to Ericsson/llvm-project that referenced this pull request Jan 13, 2025
This commit fixes three issues within the documentation file
`Annotations.rst` which was recently created by my earlier commit
llvm#122246 .

(1) The section title "Annotations to Enhance Generic Checks" is
changed to "General Purpose Annotations" because it was a bit too
verbose and it used the obsolete name "checks" for what we now call
"checkers" in the static analyzer.

(2) Several code blocks were missing from the generated html because I
accidentally used `.. code-block: c` instead of `.. code-block:: c` and
so Sphinx parsed them as comment blocks. (Without printing any error or
warning...)

(3) The `ownership_*` attributes (which are used by `MallocChecker`)
were missing from this document, so I wrote a section that briefly
describes them and links to their full documentation.
kazutakahirata pushed a commit to kazutakahirata/llvm-project that referenced this pull request Jan 13, 2025
This commit migrates the contents of 'annotations.html' in the old
HTML-based documentation of the Clang static analyzer to the new
RST-based documentation.

During this conversion I reordered the sections of this documentation
file by placing the section "Custom Assertion Handlers" as a subsection
of "Annotations to Enhance Generic Checks". (The primary motivation was
that Sphinx complained about inconsistent section levels; with this
change I preserved that sections describing individual annotations are
all on the same level.)

Apart from this change and the format conversion, I didn't review,
validate or edit the contents of this documentation file because I think
it would be better to place any additional changes in separate commits.
DKLoehr pushed a commit to DKLoehr/llvm-project that referenced this pull request Jan 17, 2025
This commit migrates the contents of 'annotations.html' in the old
HTML-based documentation of the Clang static analyzer to the new
RST-based documentation.

During this conversion I reordered the sections of this documentation
file by placing the section "Custom Assertion Handlers" as a subsection
of "Annotations to Enhance Generic Checks". (The primary motivation was
that Sphinx complained about inconsistent section levels; with this
change I preserved that sections describing individual annotations are
all on the same level.)

Apart from this change and the format conversion, I didn't review,
validate or edit the contents of this documentation file because I think
it would be better to place any additional changes in separate commits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:static analyzer clang Clang issues not falling into any other category documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants