Skip to content

[clang] Note that optnone and target attributes do not apply to nested functions #82815

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

Merged
merged 1 commit into from
May 3, 2024

Conversation

rnk
Copy link
Collaborator

@rnk rnk commented Feb 23, 2024

This behavior is true for all attributes, but this behavior can be surprising for attributes which have function-wide effects, such as optnone and target. Most other function attributes affect the prototype or semantics, but do not affect code generation in the function body. I believe it is worth calling this out in the documentation of these function-wide attributes. There may be more, these were the two that came to mind.

…d functions

This behavior is true for all attributes, but this behavior can be
surprising for attributes which have function-wide effects, such as
`optnone` and `target`. Most other function attributes affect the
prototype or semantics, but do not affect code generation in the
function body. I believe it is worth calling this out in the
documentation of these funciton-wide attributes. There may be more,
these were the two that came to mind.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 23, 2024

@llvm/pr-subscribers-clang

Author: Reid Kleckner (rnk)

Changes

This behavior is true for all attributes, but this behavior can be surprising for attributes which have function-wide effects, such as optnone and target. Most other function attributes affect the prototype or semantics, but do not affect code generation in the function body. I believe it is worth calling this out in the documentation of these function-wide attributes. There may be more, these were the two that came to mind.


Full diff: https://github.com/llvm/llvm-project/pull/82815.diff

1 Files Affected:

  • (modified) clang/include/clang/Basic/AttrDocs.td (+9)
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index b96fbddd51154c..3c58256c77201f 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2486,6 +2486,9 @@ Example "subtarget features" from the x86 backend include: "mmx", "sse", "sse4.2
 "avx", "xop" and largely correspond to the machine specific options handled by
 the front end.
 
+Note that this attribute does not apply transitively to nested functions such
+as blocks or C++ lambdas.
+
 Additionally, this attribute supports function multiversioning for ELF based
 x86/x86-64 targets, which can be used to create multiple implementations of the
 same function that will be resolved at runtime based on the priority of their
@@ -3714,6 +3717,12 @@ for that function.
 
 This attribute is incompatible with the ``always_inline`` and ``minsize``
 attributes.
+
+Note that this attribute does not apply recursively to nested functions such as
+lambdas or blocks when using declaration-specific attribute syntaxes such as double
+square brackets (``[[]]``) or ``__attribute__``. The ``#pragma`` syntax can be
+used to apply the attribute to all functions, including nested functions, in a
+range of source code.
   }];
 }
 

@rnk rnk requested review from usx95 and sam-mccall March 1, 2024 16:58
Copy link
Collaborator

@sam-mccall sam-mccall left a comment

Choose a reason for hiding this comment

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

Makes sense to me. If this ends up applying to lots of attributes then maybe we should hoist the text somewhere common, but agree that it doesn't seem to.

@cor3ntin
Copy link
Contributor

cor3ntin commented May 3, 2024

@rnk @sam-mccall Should we still merge this?

@rnk rnk merged commit 4e6d30e into llvm:main May 3, 2024
@rnk
Copy link
Collaborator Author

rnk commented May 3, 2024

Thanks for the reminder, I missed the update.

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 Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants