aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.
> I wasn't 100% whether to add the range to ParsedAttributes or even
> ParsedAttributesView.
I think `ParsedAttributesView` is the correct level for this -- I want to see
us tracking the source range for all the parsed attributes (we need the
information for diagnostics more often than not), and this ensures we always
have access to that information.
> There's still a ParsedAttributesViewWithRange which basically used once in
> ParseDeclCXX.cpp.
Any idea why we can't get rid of that one? (Maybe it'll become more clear to me
as I review more.)
================
Comment at: clang/include/clang/Parse/Parser.h:1583-1591
+ DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &attrs,
ParsingDeclSpec *DS = nullptr);
bool isDeclarationAfterDeclarator();
bool isStartOfFunctionDefinition(const ParsingDeclarator &Declarator);
- DeclGroupPtrTy ParseDeclarationOrFunctionDefinition(
- ParsedAttributesWithRange
&attrs,
- ParsingDeclSpec *DS =
nullptr,
- AccessSpecifier AS =
AS_none);
- DeclGroupPtrTy ParseDeclOrFunctionDefInternal(ParsedAttributesWithRange
&attrs,
+ DeclGroupPtrTy
+ ParseDeclarationOrFunctionDefinition(ParsedAttributes &attrs,
+ ParsingDeclSpec *DS = nullptr,
----------------
================
Comment at: clang/include/clang/Parse/Parser.h:2071
StmtResult ParseExprStatement(ParsedStmtContext StmtCtx);
- StmtResult ParseLabeledStatement(ParsedAttributesWithRange &attrs,
+ StmtResult ParseLabeledStatement(ParsedAttributes &attrs,
ParsedStmtContext StmtCtx);
----------------
================
Comment at: clang/include/clang/Parse/Parser.h:2310
SourceLocation &DeclEnd,
- ParsedAttributesWithRange &attrs,
+ ParsedAttributes &attrs,
SourceLocation *DeclSpecStart = nullptr);
----------------
================
Comment at: clang/include/clang/Parse/Parser.h:2314
ParseSimpleDeclaration(DeclaratorContext Context, SourceLocation &DeclEnd,
- ParsedAttributesWithRange &attrs, bool RequireSemi,
+ ParsedAttributes &attrs, bool RequireSemi,
ForRangeInit *FRI = nullptr,
----------------
I'll stop commenting on these -- if you see a function signature where the
parsed attribute parameter identifier uses the wrong coding style *and* the
rest of the parameters already use the right style, might as well hit those
identifiers. No need to fix all naming issues with the touched functions
(unless you feel like doing an NFC commit to change them).
================
Comment at: clang/lib/Parse/ParseDecl.cpp:500
/// a C++11 attribute in "gnu" namespace.
-void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
- SourceLocation AttrNameLoc,
- ParsedAttributes &Attrs,
- SourceLocation *EndLoc,
- IdentifierInfo *ScopeName,
- SourceLocation ScopeLoc,
- ParsedAttr::Syntax Syntax,
- Declarator *D) {
+void Parser::ParseGNUAttributeArgs(
+ IdentifierInfo *AttrName, SourceLocation AttrNameLoc,
----------------
It looks like some unrelated formatting changes snuck in? I'm not opposed, but
it's a bit easier to review if this was an NFC commit (before or after landing
this patch) because it makes for a smaller diff here. (I noticed this in a few
spots, only commenting about it here.)
================
Comment at: clang/test/SemaOpenCL/address-spaces.cl:261
typedef __private int private_int_t;
- __private __attribute__((opencl_global)) int var1; // expected-error
{{multiple address spaces specified for type}} \
+ __attribute__((opencl_global)) __private int var1; // expected-error
{{multiple address spaces specified for type}} \
// expected-error
{{function scope variable cannot be declared in global address space}}
----------------
tbaeder wrote:
> This is a peculiar ordering problem...
I think we need to ensure that we still diagnose both ways, otherwise we'll
start to accept invalid code: https://godbolt.org/z/h7473Ehc5
Or does the behavior change here in other ways?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121201/new/
https://reviews.llvm.org/D121201
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
