Arguably as a bug, Clang has previously not mixed up Objective-C
parameter names with types. This allows developers to write parameter
names that _should_ shadow type names, but don't. For instance:
@interface Foo
-(void)foo:(int)id bar:(id)name; // OK
@end
Commit 97788089988a2ace63d717cadbcfe3443f380f9c changed the way that
parameters are parsed to bring it more in line with how C parameters are
parsed, but it breaks the example above. Given an expectation that the
change wouldn't introduce source breaks, this is not something we can go
forward with.
97788089988a2ace63d717cadbcfe3443f380f9c did this so that late-parsed
attributes could reference Objective-C parameters. This change buffers
Objective-C parameter info until after all parameters are parsed and
turns them into parameter declarations before realizing late-parsed
attributes instead.
Radar-ID: 139996306
Before this change, ParseObjc would call the closing
`MaybeParseAttributes` before it had created Objective-C `ParmVarDecl`
objects (and associated name lookup entries), meaning that you could not
reference Objective-C method parameters in
`__attribute__((diagnose_if))`. This change moves the creation of the
`ParmVarDecl` objects ahead of calling `Sema::ActOnMethodDeclaration` so
that `MaybeParseAttributes` can find them. This is already how it works
for C parameters hanging off of the selector.
This change alone is insufficient to enable `diagnose_if` for
Objective-C methods and effectively is NFC. There will be a follow-up PR
for diagnose_if. This change is still useful for any other work that may
need attributes to reference Objective-C parameters.
rdar://138596211
When various `Sema*.h` and `Sema*.cpp` files were created, cleanup of
`Sema.h` includes and forward declarations was left for the later.
Now's the time. This commit touches `Sema.h` and Sema components:
1. Unused includes are removed.
2. Unused forward declarations are removed.
3. Missing includes are added (those files are largely IWYU-clean now).
4. Includes were converted into forward declarations where possible.
As this commit focuses on headers, all changes to `.cpp` files were
minimal, and were aiming at keeping everything buildable.
[BoundsSafety] Reland #93121 Allow 'counted_by' attribute on pointers in structs in C (#93121)
Fixes#92687.
Previously the attribute was only allowed on flexible array members.
This patch patch changes this to also allow the attribute on pointer
fields in structs and also allows late parsing of the attribute in some
contexts.
For example this previously wasn't allowed:
```
struct BufferTypeDeclAttributePosition {
size_t count;
char* buffer __counted_by(count); // Now allowed
}
```
Note the attribute is prevented on pointee types where the size isn't
known at compile time. In particular pointee types that are:
* Incomplete (e.g. `void`) and sizeless types
* Function types (e.g. the pointee of a function pointer)
* Struct types with a flexible array member
This patch also introduces late parsing of the attribute when used in
the declaration attribute position. For example
```
struct BufferTypeDeclAttributePosition {
char* buffer __counted_by(count); // Now allowed
size_t count;
}
```
is now allowed but **only** when passing
`-fexperimental-late-parse-attributes`. The motivation for using late
parsing here is to avoid breaking the data layout of structs in existing
code that want to use the `counted_by` attribute. This patch is the
first use of `LateAttrParseExperimentalExt` in `Attr.td` that was
introduced in a previous patch.
Note by allowing the attribute on struct member pointers this now allows
the possiblity of writing the attribute in the type attribute position.
For example:
```
struct BufferTypeAttributePosition {
size_t count;
char *__counted_by(count) buffer; // Now allowed
}
```
However, the attribute in this position is still currently parsed
immediately rather than late parsed. So this will not parse currently:
```
struct BufferTypeAttributePosition {
char *__counted_by(count) buffer; // Fails to parse
size_t count;
}
```
The intention is to lift this restriction in future patches. It has not
been done in this patch to keep this size of this commit small.
There are also several other follow up changes that will need to be
addressed in future patches:
* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).
This work is heavily based on a patch originally written by Yeoul Na.
** Differences between #93121 and this patch **
* The memory leak that caused #93121 to be reverted (see #92687) should
now be fixed. See "The Memory Leak".
* The fix to `pragma-attribute-supported-attributes-list.test`
(originally in cef6387) has been incorporated into this patch.
* A relaxation of counted_by semantics (originally in 112eadd) has been
incorporated into this patch.
* The assert in `Parser::DistributeCLateParsedAttrs` has been removed
because that broke downstream code.
* The switch statement in `Parser::ParseLexedCAttribute` has been
removed in favor of using `Parser::ParseGNUAttributeArgs` which does
the same thing but is more feature complete.
* The `EnterScope` parameter has been plumbed through
`Parser::ParseLexedCAttribute` and `Parser::ParseLexedCAttributeList`.
It currently doesn't do anything but it will be needed in future
commits.
** The Memory Leak **
The problem was that these lines parsed the attributes but then did nothing to free the memory
```
assert(!getLangOpts().CPlusPlus);
for (auto *LateAttr : LateFieldAttrs)
ParseLexedCAttribute(*LateAttr);
```
To fix this this a new `Parser::ParseLexedCAttributeList` method has been
added (based on `Parser::ParseLexedAttributeList`) which does the
necessary memory management. The intention is to merge these two
methods together so there is just one implementation in a future patch
(#93263).
A more principled fixed here would be to fix the ownership of the
`LateParsedAttribute` objects. In principle `LateParsedAttrList` should own
its pointers exclusively and be responsible for deallocating them.
Unfortunately this is complicated by `LateParsedAttribute` objects also
being stored in another data structure (`LateParsedDeclarations`) as
can be seen below (`LA` gets stored in two places).
```
// Handle attributes with arguments that require late parsing.
LateParsedAttribute *LA =
new LateParsedAttribute(this, *AttrName, AttrNameLoc);
LateAttrs->push_back(LA);
// Attributes in a class are parsed at the end of the class, along
// with other late-parsed declarations.
if (!ClassStack.empty() && !LateAttrs->parseSoon())
getCurrentClass().LateParsedDeclarations.push_back(LA);
```
this means the ownership of LateParsedAttribute objects isn't very
clear.
rdar://125400257
Previously the attribute was only allowed on flexible array members.
This patch patch changes this to also allow the attribute on pointer
fields in structs and also allows late parsing of the attribute in some
contexts.
For example this previously wasn't allowed:
```
struct BufferTypeDeclAttributePosition {
size_t count;
char* buffer __counted_by(count); // Now allowed
}
```
Note the attribute is prevented on pointee types where the size isn't
known at compile time. In particular pointee types that are:
* Incomplete (e.g. `void`) and sizeless types
* Function types (e.g. the pointee of a function pointer)
* Struct types with a flexible array member
This patch also introduces late parsing of the attribute when used in
the declaration attribute position. For example
```
struct BufferTypeDeclAttributePosition {
char* buffer __counted_by(count); // Now allowed
size_t count;
}
```
is now allowed but **only** when passing
`-fexperimental-late-parse-attributes`. The motivation for using late
parsing here is to avoid breaking the data layout of structs in existing
code that want to use the `counted_by` attribute. This patch is the
first use of `LateAttrParseExperimentalExt` in `Attr.td` that was
introduced in a previous patch.
Note by allowing the attribute on struct member pointers this now allows
the possiblity of writing the attribute in the type attribute position.
For example:
```
struct BufferTypeAttributePosition {
size_t count;
char *__counted_by(count) buffer; // Now allowed
}
```
However, the attribute in this position is still currently parsed
immediately rather than late parsed. So this will not parse currently:
```
struct BufferTypeAttributePosition {
char *__counted_by(count) buffer; // Fails to parse
size_t count;
}
```
The intention is to lift this restriction in future patches. It has not
been done in this patch to keep this size of this commit small.
There are also several other follow up changes that will need to be
addressed in future patches:
* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).
This work is heavily based on a patch originally written by Yeoul Na.
rdar://125400257
Co-authored-by: Dan Liew <dan@su-root.co.uk>
This patch continues previous efforts to split `Sema` up, this time
covering code completion.
Context can be found in #84184.
Dropping `Code` prefix from function names in `SemaCodeCompletion` would
make sense, but I think this PR has enough changes already.
As usual, formatting changes are done as a separate commit. Hopefully
this helps with the review.
Attribute `optnone` must turn off all optimizations including fast-math
ones. Actually AST nodes in the 'optnone' function still had fast-math
flags. This change implements fixing FP options before function body is
parsed.
- Sema::isSimpleTypeSpecifier return true for _Bool in c99 (currently
returns false for _Bool, regardless of C dialect). (Fixes#72203)
- replace the logic with a check for simple types and a proper check for
a valid keyword in the appropriate dialect
Co-authored-by: Carl Peto <CPeto@becrypt.com>
This prevents further parsing of tokens (that'll be freed) inside method
body by propagating EOF emitted by reaching code completion token up the parsing
stack.
Differential Revision: https://reviews.llvm.org/D158269
- Use this new context in Sema to limit completions to seen ObjC class
names
- Use this new context in clangd to disable include insertions when
completing ObjC forward decls
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D150978
This patch adds static functions for constructing most
AttributeCommonInfo::Forms. Direct construction is only retained where
all fields (currently the syntax and spelling) are specified explicitly.
This is a wash on its own. The purpose is to allow extra fields
to be added to Form without disrupting all callers. In particular,
it allows extra information to be stored about keywords without
affecting non-keyword uses.
No functional change intended.
Differential Revision: https://reviews.llvm.org/D148104
Instead of emitting a redefinition error, check that definitions are
equivalent and allow such scenario.
A few non-obvious implementation details:
* to avoid multiple definitions in the redeclaration chain we just drop
the new definition after checking for equivalence;
* for checking definition equivalence use ODR hash instead of
ASTStructuralEquivalence because it avoids excessive recursive
deserialization. Though after detecting a mismatch we do deserialize
multiple entities to provide a better error message.
rdar://82908223
Differential Revision: https://reviews.llvm.org/D124286
This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
We would previously reject valid input where GNU attributes preceded the
standard attributes on top-level declarations. A previous attribute
handling change had begun rejecting this whilst GCC does honour this
layout. In practice, this breaks use of `extern "C"` attributed
functions which use both standard and GNU attributes as experienced by
the Swift runtime.
Objective-C deserves an honourable mention for requiring some additional
special casing. Because attributes on declarations and definitions
differ in semantics, we need to replicate some of the logic for
detecting attributes to declarations to which they appertain cannot be
attributed. This should match the existing case for the application of
GNU attributes to interfaces, protocols, and implementations.
Take the opportunity to split out the tooling tests into two cases: ones
which process macros and ones which do not.
Special thanks to Aaron Ballman for the many hints and extensive rubber
ducking that was involved in identifying the various places where we
accidentally dropped attributes.
Differential Revision: https://reviews.llvm.org/D137979Fixes: #58229
Reviewed By: aaron.ballman, arphaman
When during parsing we encountered a duplicate `ObjCProtocolDecl`, we
were always emitting an error. With this change we accept
* when a previous `ObjCProtocolDecl` is in a hidden [sub]module;
* parsed `ObjCProtocolDecl` is the same as the previous one.
And in case of mismatches we provide more detailed error messages.
rdar://93069080
Differential Revision: https://reviews.llvm.org/D130327
For backwards compatiblity, we emit only a warning instead of an error if the
attribute is one of the existing type attributes that we have historically
allowed to "slide" to the `DeclSpec` just as if it had been specified in GNU
syntax. (We will call these "legacy type attributes" below.)
The high-level changes that achieve this are:
- We introduce a new field `Declarator::DeclarationAttrs` (with appropriate
accessors) to store C++11 attributes occurring in the attribute-specifier-seq
at the beginning of a simple-declaration (and other similar declarations).
Previously, these attributes were placed on the `DeclSpec`, which made it
impossible to reconstruct later on whether the attributes had in fact been
placed on the decl-specifier-seq or ahead of the declaration.
- In the parser, we propgate declaration attributes and decl-specifier-seq
attributes separately until we can place them in
`Declarator::DeclarationAttrs` or `DeclSpec::Attrs`, respectively.
- In `ProcessDeclAttributes()`, in addition to processing declarator attributes,
we now also process the attributes from `Declarator::DeclarationAttrs` (except
if they are legacy type attributes).
- In `ConvertDeclSpecToType()`, in addition to processing `DeclSpec` attributes,
we also process any legacy type attributes that occur in
`Declarator::DeclarationAttrs` (and emit a warning).
- We make `ProcessDeclAttribute` emit an error if it sees any non-declaration
attributes in C++11 syntax, except in the following cases:
- If it is being called for attributes on a `DeclSpec` or `DeclaratorChunk`
- If the attribute is a legacy type attribute (in which case we only emit
a warning)
The standard justifies treating attributes at the beginning of a
simple-declaration and attributes after a declarator-id the same. Here are some
relevant parts of the standard:
- The attribute-specifier-seq at the beginning of a simple-declaration
"appertains to each of the entities declared by the declarators of the
init-declarator-list" (https://eel.is/c++draft/dcl.dcl#dcl.pre-3)
- "In the declaration for an entity, attributes appertaining to that entity can
appear at the start of the declaration and after the declarator-id for that
declaration." (https://eel.is/c++draft/dcl.dcl#dcl.pre-note-2)
- "The optional attribute-specifier-seq following a declarator-id appertains to
the entity that is declared."
(https://eel.is/c++draft/dcl.dcl#dcl.meaning.general-1)
The standard contains similar wording to that for a simple-declaration in other
similar types of declarations, for example:
- "The optional attribute-specifier-seq in a parameter-declaration appertains to
the parameter." (https://eel.is/c++draft/dcl.fct#3)
- "The optional attribute-specifier-seq in an exception-declaration appertains
to the parameter of the catch clause" (https://eel.is/c++draft/except.pre#1)
The new behavior is tested both on the newly added type attribute
`annotate_type`, for which we emit errors, and for the legacy type attribute
`address_space` (chosen somewhat randomly from the various legacy type
attributes), for which we emit warnings.
Depends On D111548
Reviewed By: aaron.ballman, rsmith
Differential Revision: https://reviews.llvm.org/D126061
Without the fix ivars with anonymous types can trigger errors like
> error: 'TestClass::structIvar' from module 'Target' is not present in definition of 'TestClass' provided earlier
> [...]
> note: declaration of 'structIvar' does not match
It happens because types of ivars from different modules are considered
to be different. And it is caused by not merging anonymous `TagDecl`
from different modules.
To fix that I've changed `serialization::needsAnonymousDeclarationNumber`
to handle anonymous `TagDecl` inside `ObjCInterfaceDecl`. But that's not
sufficient as C code inside `ObjCInterfaceDecl` doesn't use interface
decl as a decl context but switches to its parent (TranslationUnit in
most cases). I'm changing that to make `ObjCContainerDecl` the lexical
decl context but keeping the semantic decl context intact.
Test "check-dup-decls-inside-objc.m" doesn't reflect a change in
functionality but captures the existing behavior to prevent regressions.
rdar://85563013
Differential Revision: https://reviews.llvm.org/D118525
Move the SourceRange from the old ParsedAttributesWithRange into
ParsedAttributesView, so we have source range information available
everywhere we use attributes.
This also removes ParsedAttributesWithRange (replaced by simply using
ParsedAttributes) and ParsedAttributesVieWithRange (replaced by using
ParsedAttributesView).
Differential Revision: https://reviews.llvm.org/D121201
In C++20 modules imports must be together and at the start of the module.
Rather than growing more ad-hoc flags to test state, this keeps track of the
phase of of a valid module TU (first decl, global module frag, module,
private module frag). If the phasing is broken (with some diagnostic) the
pattern does not conform to a valid C++20 module, and we set the state
accordingly.
We can thus issue diagnostics when imports appear in the wrong places and
decouple the C++20 modules state from other module variants (modules-ts and
clang modules). Additionally, we attempt to diagnose wrong imports before
trying to find the module where possible (the latter will generally emit an
unhelpful diagnostic about the module not being available).
Although this generally simplifies the handling of C++20 module import
diagnostics, the motivation was that, in particular, it allows detecting
invalid imports like:
import module A;
int some_decl();
import module B;
where being in a module purview is insufficient to identify them.
Differential Revision: https://reviews.llvm.org/D118893
In C++20 modules imports must be together and at the start of the module.
Rather than growing more ad-hoc flags to test state, this keeps track of the
phase of of a valid module TU (first decl, global module frag, module,
private module frag). If the phasing is broken (with some diagnostic) the
pattern does not conform to a valid C++20 module, and we set the state
accordingly.
We can thus issue diagnostics when imports appear in the wrong places and
decouple the C++20 modules state from other module variants (modules-ts and
clang modules). Additionally, we attempt to diagnose wrong imports before
trying to find the module where possible (the latter will generally emit an
unhelpful diagnostic about the module not being available).
Although this generally simplifies the handling of C++20 module import
diagnostics, the motivation was that, in particular, it allows detecting
invalid imports like:
import module A;
int some_decl();
import module B;
where being in a module purview is insufficient to identify them.
Differential Revision: https://reviews.llvm.org/D118893
Somewhat surprisingly, signature help is emitted as a side-effect of
computing the expected type of a function argument.
The reason is that both actions require enumerating the possible
function signatures and running partial overload resolution, and doing
this twice would be wasteful and complicated.
Change #1: document this, it's subtle :-)
However, sometimes we need to compute the expected type without having
reached the code completion cursor yet - in particular to allow
completion of designators.
eb4ab3358cd4dc834a761191b5531b38114f7b13 did this but introduced a
regression - it emits signature help in the wrong location as a side-effect.
Change #2: only emit signature help if the code completion cursor was reached.
Currently there is PP.isCodeCompletionReached(), but we can't use it
because it's set *after* running code completion.
It'd be nice to set this implicitly when the completion token is lexed,
but ConsumeCodeCompletionToken() makes this complicated.
Change #3: call cutOffParsing() *first* when seeing a completion token.
After this, the fact that the Sema::Produce*SignatureHelp() functions
are even more confusing, as they only sometimes do that.
I don't want to rename them in this patch as it's another large
mechanical change, but we should soon.
Change #4: prepare to rename ProduceSignatureHelp() to GuessArgumentType() etc.
Differential Revision: https://reviews.llvm.org/D98488
In Clang today, we parse the different attribute syntaxes
(__attribute__, __declspec, and [[]]) in a fairly rigid order. This
leads to confusion for users when they guess the order incorrectly,
and leads to bug reports like PR24559 or necessitates changes like
D94788.
This patch adds a helper function to allow us to more easily parse
attributes in arbitrary order, and then updates all of the places
where we would parse two or more different syntaxes in a rigid order to
use the helper method. The patch does not attempt to handle Microsoft
attributes ([]) because those are ambiguous with other code constructs
and we don't have any attributes that use the syntax.
Since these are scoped enumerators, they have to be prefixed by DeclaratorContext, so lets remove Context from the name, and return some characters to the multiverse.
Patch was reviewed here: https://reviews.llvm.org/D91011
Thank you to aaron, bruno, wyatt and barry for indulging me.
This is a code clean up of the PropertyAttributeKind and
ObjCPropertyAttributeKind enums in ObjCPropertyDecl and ObjCDeclSpec that are
exactly identical. This non-functional change consolidates these enums
into one. The changes are to many files across clang (and comments in LLVM) so
that everything refers to the new consolidated enum in DeclObjCCommon.h.
2nd Landing Attempt...
Differential Revision: https://reviews.llvm.org/D77233
This is a code clean up of the PropertyAttributeKind and
ObjCPropertyAttributeKind enums in ObjCPropertyDecl and ObjCDeclSpec that are
exactly identical. This non-functional change consolidates these enums
into one. The changes are to many files across clang (and comments in LLVM) so
that everything refers to the new consolidated enum in DeclObjCCommon.h.
Differential Revision: https://reviews.llvm.org/D77233
__attribute__((objc_direct)) is an attribute on methods declaration, and
__attribute__((objc_direct_members)) on implementation, categories or
extensions.
A `direct` property specifier is added (@property(direct) type name)
These attributes / specifiers cause the method to have no associated
Objective-C metadata (for the property or the method itself), and the
calling convention to be a direct C function call.
The symbol for the method has enforced hidden visibility and such direct
calls are hence unreachable cross image. An explicit C function must be
made if so desired to wrap them.
The implicit `self` and `_cmd` arguments are preserved, however to
maintain compatibility with the usual `objc_msgSend` semantics,
3 fundamental precautions are taken:
1) for instance methods, `self` is nil-checked. On arm64 backends this
typically adds a single instruction (cbz x0, <closest-ret>) to the
codegen, for the vast majority of the cases when the return type is a
scalar.
2) for class methods, because the class may not be realized/initialized
yet, a call to `[self self]` is emitted. When the proper deployment
target is used, this is optimized to `objc_opt_self(self)`.
However, long term we might want to emit something better that the
optimizer can reason about. When inlining kicks in, these calls
aren't optimized away as the optimizer has no idea that a single call
is really necessary.
3) the calling convention for the `_cmd` argument is changed: the caller
leaves the second argument to the call undefined, and the selector is
loaded inside the body when it's referenced only.
As far as error reporting goes, the compiler refuses:
- making any overloads direct,
- making an overload of a direct method,
- implementations marked as direct when the declaration in the
interface isn't (the other way around is allowed, as the direct
attribute is inherited from the declaration),
- marking methods required for protocol conformance as direct,
- messaging an unqualified `id` with a direct method,
- forming any @selector() expression with only direct selectors.
As warnings:
- any inconsistency of direct-related calling convention when
@selector() or messaging is used,
- forming any @selector() expression with a possibly direct selector.
Lastly an `objc_direct_members` attribute is added that can decorate
`@implementation` blocks and causes methods only declared there (and in
no `@interface`) to be automatically direct. When decorating an
`@interface` then all methods and properties declared in this block are
marked direct.
Radar-ID: rdar://problem/2684889
Differential Revision: https://reviews.llvm.org/D69991
Reviewed-By: John McCall
If the type didn't exist, we used to emit a really bad error:
t.m:3:12: error: expected ')'
-(nullable NoSuchType)foo3;
^
rdar://50925632
llvm-svn: 364489
Summary:
By adding a hook to consume all tokens produced by the preprocessor.
The intention of this change is to make it possible to consume the
expanded tokens without re-runnig the preprocessor with minimal changes
to the preprocessor and minimal performance penalty when preprocessing
without recording the tokens.
The added hook is very low-level and reconstructing the expanded token
stream requires more work in the client code, the actual algorithm to
collect the tokens using this hook can be found in the follow-up change.
Reviewers: rsmith
Reviewed By: rsmith
Subscribers: eraman, nemanjai, kbarton, jsji, riccibruno, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59885
llvm-svn: 361007
We want to make objc_nonlazy_class apply to implementations, but ran into this.
There doesn't seem to be any reason that this isn't supported.
Differential revision: https://reviews.llvm.org/D60542
llvm-svn: 358200
This adds support for static_assert() (and _Static_assert()) in
@interface/@implementation ivar lists and in @interface method declarations.
It was already supported in @implementation blocks outside of the ivar lists.
The assert AST nodes are added at file scope, matching where other
(non-Objective-C) declarations at @interface / @implementation level go (cf
`allTUVariables`).
Also add a `__has_feature(objc_c_static_assert)` that's true in C11 (and
`__has_extension(objc_c_static_assert)` that's always true) and
`__has_feature(objc_cxx_static_assert)` that's true in C++11 modea fter this
patch, so it's possible to check if this is supported.
Differential Revision: https://reviews.llvm.org/D59223
llvm-svn: 356148
