[PATCH] D50526: Model type attributes as regular Attrs
rnk added a comment.
The issue was related to ignored calling convention attributes on Win64:
int a(int, const int *, int, int, __int64);
class b {
public:
typedef int c;
};
template
void AtlAxDialogCreateT(int, d, int, int, __int64);
int g, i, j, k;
char *h;
void l() { AtlAxDialogCreateT(g, h, i, j, k); }
Compiled with:
clang -c -fms-extensions -fms-compatibility --target=x86_64-windows-msvc
Repository:
rC Clang
https://reviews.llvm.org/D50526
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50526: Model type attributes as regular Attrs
rnk added a comment. I went ahead and reverted this in https://reviews.llvm.org/rC339638, and will produce a reduction soon. Repository: rC Clang https://reviews.llvm.org/D50526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50526: Model type attributes as regular Attrs
rnk added a comment. I suspect that somehow this change broke compiling ATL: https://ci.chromium.org/buildbot/chromium.clang/ToTWin64%28dbg%29/993 https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.clang%2FToTWin64_dbg_%2F995%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Flogs%2Fraw_io.output_failure_summary_%2F0 Now we get these diagnostics: [1011/50166] CXX obj/chrome/chrome_cleaner/http/http/error_utils.obj FAILED: obj/chrome/chrome_cleaner/http/http/error_utils.obj ... In file included from ../../chrome/chrome_cleaner/http/error_utils.cc:11: In file included from ../..\base/win/atl.h:18: In file included from ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlctl.h:33: In file included from ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlwin.h:5107: ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2777,9): error: no matching function for call to 'AtlAxDialogCreateT' return AtlAxDialogCreateT( ^~~~ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2715,29): note: candidate template ignored: invalid explicitly-specified argument for template parameter 'pFunc' typename Helper::ReturnType AtlAxDialogCreateT( ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2788,9): error: no matching function for call to 'AtlAxDialogCreateT' return AtlAxDialogCreateT( ^~~ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2715,29): note: candidate template ignored: invalid explicitly-specified argument for template parameter 'pFunc' typename Helper::ReturnType AtlAxDialogCreateT( ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2799,9): error: no matching function for call to 'AtlAxDialogCreateT' return AtlAxDialogCreateT( ^~~~ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2715,29): note: candidate template ignored: invalid explicitly-specified argument for template parameter 'pFunc' typename Helper::ReturnType AtlAxDialogCreateT( ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2810,9): error: no matching function for call to 'AtlAxDialogCreateT' return AtlAxDialogCreateT(hInstance, lpTemplateName, hWndParent, lpDialogProc, dwInitParam); ^~~ ..\..\third_party\depot_tools\win_toolchain\vs_files\3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c\VC\Tools\MSVC\14.14.26428\atlmfc\include\atlhost.h(2715,29): note: candidate template ignored: invalid explicitly-specified argument for template parameter 'pFunc' typename Helper::ReturnType AtlAxDialogCreateT( ^ 4 errors generated. Repository: rC Clang https://reviews.llvm.org/D50526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50526: Model type attributes as regular Attrs
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM! Thank you for this fantastic work!
Comment at: include/clang/Basic/Attr.td:1510
+ let Spellings = [Keyword<"__unsafe_unretained">];
+ let Documentation = [Undocumented];
+}
rsmith wrote:
> aaron.ballman wrote:
> > I don't suppose you can throw in some quick docs for this keyword? Or is
> > this not really user-facing? If it's not user-facing, perhaps it should
> > have no spellings and only ever be created implicitly?
> This isn't actually the primary representation of `__unsafe_unretained`, this
> is an internal placeholder for "the user wrote `__unsafe_unretained` but
> we're not in ARC mode so it's meaningless (hence "inert")". So I don't think
> this is where we should attach the documentation (the right place is the
> `ObjCOwnership` attribute, which is also currently `Undocumented`, sadly).
> I'll at least add a comment to the .td file to explain that.
Ah, thank you for the explanation; I agree.
Comment at: lib/AST/Type.cpp:1624
+ const Type *Cur = this;
+ while (auto *AT = Cur->getAs()) {
+if (AT->getAttrKind() == AK)
rsmith wrote:
> aaron.ballman wrote:
> > `const auto *`
> Done, but...
>
> The pointee type is deduced as `const` anyway, so the `const` doesn't give
> any extra type safety. So the only benefit would be to the reader, and I
> don't think a reader of this code would care whether `*AT` is mutable, so the
> `const` seems like a distraction to me (and hence the explicit `const` is a
> minor harm to readability rather than a minor improvement). I can see how a
> reader trained to think that absence-of-`const` implies that mutation is
> intended would find the explicit `const` clearer, but (for better or probably
> worse) that's not the style we generally use in Clang (for instance, I didn't
> mark the `AK` parameter as `const`, but there is no implication that I intend
> to modify it).
>
> That said, I don't feel strongly about this, and I certainly have no
> objection to making the change here. If we generally want to move to a style
> where `auto` is always accompanied by an explicit `const` when `const` would
> be deduced (in much the same way that we already expect that `auto` be
> accompanied by an explicit `*` when a pointer type would be deduced), I'd be
> OK with that -- but we should discuss that somewhere more visible than this
> review thread and include it in our general coding guidelines.
> That said, I don't feel strongly about this, and I certainly have no
> objection to making the change here. If we generally want to move to a style
> where auto is always accompanied by an explicit const when const would be
> deduced (in much the same way that we already expect that auto be accompanied
> by an explicit * when a pointer type would be deduced), I'd be OK with that
> -- but we should discuss that somewhere more visible than this review thread
> and include it in our general coding guidelines.
Yeah, I actually thought this already was part of the coding guidelines, truth
be told. But I went and looked again and realized we only talk about `*` and
`&` being deduced, not qualifiers. I guess my preference has always been to
explicitly spell out pertinent information about the type beyond the name, such
as whether it's `const`, whether it's a pointer, etc. Basically, things that
aren't immediately obvious from the context.
I prefer being explicit about spelling out the `const` here because it makes it
obvious that the type is not intended to be mutated, but I only really care
about it in cases where the type is deduced to a pointer or reference (and so
mutating operations might be hidden behind a function call that looks harmless).
Perhaps this is worth starting a coding guideline discussion over?
Comment at: lib/Sema/SemaType.cpp:3884
+template
+static AttrT *createSimpleAttr(ASTContext , ParsedAttr ) {
rsmith wrote:
> aaron.ballman wrote:
> > Did clang-format do this? It seems like it's not formatted how I'd expect.
> How would you expect it? clang-format puts a space after the `template`
> keyword, unfortunately, and IIRC can't be configured to not do so. Though as
> a consequence, it looks like the space is now more common in clang code by a
> 2:1 ratio despite being "clearly wrong" ;-(
I was expecting to see a space after `template` as I thought that was the most
common form of it in the code base. :-D
Comment at: lib/Sema/SemaType.cpp:6360
+ }
+}
+
rsmith wrote:
> aaron.ballman wrote:
> > MSVC may complain about not all control paths returning a value here.
> I'm confident that this pattern is fine; we use it all over the place. MSVC
> knows that control flow cannot leave an `llvm_unreachable(...)`.
Yeah, I think I was mis-remembering a pattern that
[PATCH] D50526: Model type attributes as regular Attrs
rsmith added inline comments.
Comment at: include/clang/Basic/Attr.td:1510
+ let Spellings = [Keyword<"__unsafe_unretained">];
+ let Documentation = [Undocumented];
+}
aaron.ballman wrote:
> I don't suppose you can throw in some quick docs for this keyword? Or is this
> not really user-facing? If it's not user-facing, perhaps it should have no
> spellings and only ever be created implicitly?
This isn't actually the primary representation of `__unsafe_unretained`, this
is an internal placeholder for "the user wrote `__unsafe_unretained` but we're
not in ARC mode so it's meaningless (hence "inert")". So I don't think this is
where we should attach the documentation (the right place is the
`ObjCOwnership` attribute, which is also currently `Undocumented`, sadly). I'll
at least add a comment to the .td file to explain that.
Comment at: lib/AST/Type.cpp:1624
+ const Type *Cur = this;
+ while (auto *AT = Cur->getAs()) {
+if (AT->getAttrKind() == AK)
aaron.ballman wrote:
> `const auto *`
Done, but...
The pointee type is deduced as `const` anyway, so the `const` doesn't give any
extra type safety. So the only benefit would be to the reader, and I don't
think a reader of this code would care whether `*AT` is mutable, so the `const`
seems like a distraction to me (and hence the explicit `const` is a minor harm
to readability rather than a minor improvement). I can see how a reader trained
to think that absence-of-`const` implies that mutation is intended would find
the explicit `const` clearer, but (for better or probably worse) that's not the
style we generally use in Clang (for instance, I didn't mark the `AK` parameter
as `const`, but there is no implication that I intend to modify it).
That said, I don't feel strongly about this, and I certainly have no objection
to making the change here. If we generally want to move to a style where `auto`
is always accompanied by an explicit `const` when `const` would be deduced (in
much the same way that we already expect that `auto` be accompanied by an
explicit `*` when a pointer type would be deduced), I'd be OK with that -- but
we should discuss that somewhere more visible than this review thread and
include it in our general coding guidelines.
Comment at: lib/AST/Type.cpp:3215
}
- llvm_unreachable("bad attributed type kind");
}
aaron.ballman wrote:
> Probably need to keep this to prevent MSVC from barking about not all control
> paths returning a value.
There's a `default:` case now, so that shouldn't be a problem. (Removing the
exhaustive list is intended to be temporary; I'd like to move to generating
this with TableGen sooner rather than later.)
Comment at: lib/Sema/SemaType.cpp:178-180
+// their TypeLocs makes it hard to correctly assign these. We use the
+// keep the attributes in creation order as an attempt to make them
+// line up properly.
aaron.ballman wrote:
> "We use the keep the attributes" isn't grammatical. Should it be, "We keep
> the attributes in creation order" instead?
Hah, oops, yes.
Comment at: lib/Sema/SemaType.cpp:181
+// line up properly.
+using TypeAttr = std::pair;
+SmallVector TypeAttrs;
aaron.ballman wrote:
> It's unfortunate to use the name `TypeAttr` here -- unqualified uses of the
> type name, like below, makes it look like it might be the `TypeAttr` from
> Attr.h
Good point. I went with `TypeAttrPair` here, and `AttrsForTypes` in the vector.
Comment at: lib/Sema/SemaType.cpp:3884
+template
+static AttrT *createSimpleAttr(ASTContext , ParsedAttr ) {
aaron.ballman wrote:
> Did clang-format do this? It seems like it's not formatted how I'd expect.
How would you expect it? clang-format puts a space after the `template`
keyword, unfortunately, and IIRC can't be configured to not do so. Though as a
consequence, it looks like the space is now more common in clang code by a 2:1
ratio despite being "clearly wrong" ;-(
Comment at: lib/Sema/SemaType.cpp:6360
+ }
+}
+
aaron.ballman wrote:
> MSVC may complain about not all control paths returning a value here.
I'm confident that this pattern is fine; we use it all over the place. MSVC
knows that control flow cannot leave an `llvm_unreachable(...)`.
Comment at: lib/Serialization/ASTReaderDecl.cpp:2707
void ASTReader::ReadAttributes(ASTRecordReader , AttrVec ) {
- for (unsigned i = 0, e = Record.readInt(); i != e; ++i) {
-Attr *New = nullptr;
-auto Kind = (attr::Kind)Record.readInt();
-SourceRange Range = Record.readSourceRange();
-ASTContext = getContext();
-
-#include "clang/Serialization/AttrPCHRead.inc"
-
-assert(New && "Unable to decode attribute?");
-Attrs.push_back(New);
- }
+ for (unsigned i = 0, e = Record.readInt(); i != e;
[PATCH] D50526: Model type attributes as regular Attrs
rsmith updated this revision to Diff 160207.
rsmith marked 10 inline comments as done.
https://reviews.llvm.org/D50526
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Attr.h
include/clang/AST/Type.h
include/clang/AST/TypeLoc.h
include/clang/Basic/Attr.td
include/clang/Sema/Sema.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/ASTWriter.h
lib/ARCMigrate/TransGCAttrs.cpp
lib/ARCMigrate/Transforms.cpp
lib/AST/ASTContext.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/AST/TypePrinter.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaInit.cpp
lib/Sema/SemaObjCProperty.cpp
lib/Sema/SemaType.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
lib/StaticAnalyzer/Core/CheckerHelpers.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2470,8 +2470,10 @@
static const AttrClassDescriptor AttrClassDescriptors[] = {
{ "ATTR", "Attr" },
+ { "TYPE_ATTR", "TypeAttr" },
{ "STMT_ATTR", "StmtAttr" },
{ "INHERITABLE_ATTR", "InheritableAttr" },
+ { "DECL_OR_TYPE_ATTR", "DeclOrTypeAttr" },
{ "INHERITABLE_PARAM_ATTR", "InheritableParamAttr" },
{ "PARAMETER_ABI_ATTR", "ParameterABIAttr" }
};
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -103,9 +103,9 @@
const auto *AttrType = Type->getAs();
if (!AttrType)
return Nullability::Unspecified;
- if (AttrType->getAttrKind() == AttributedType::attr_nullable)
+ if (AttrType->getAttrKind() == attr::TypeNullable)
return Nullability::Nullable;
- else if (AttrType->getAttrKind() == AttributedType::attr_nonnull)
+ else if (AttrType->getAttrKind() == attr::TypeNonNull)
return Nullability::Nonnull;
return Nullability::Unspecified;
}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -770,19 +770,7 @@
}
void TypeLocWriter::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
- Record.AddSourceLocation(TL.getAttrNameLoc());
- if (TL.hasAttrOperand()) {
-SourceRange range = TL.getAttrOperandParensRange();
-Record.AddSourceLocation(range.getBegin());
-Record.AddSourceLocation(range.getEnd());
- }
- if (TL.hasAttrExprOperand()) {
-Expr *operand = TL.getAttrExprOperand();
-Record.push_back(operand ? 1 : 0);
-if (operand) Record.AddStmt(operand);
- } else if (TL.hasAttrEnumOperand()) {
-Record.AddSourceLocation(TL.getAttrEnumOperandLoc());
- }
+ Record.AddAttr(TL.getAttr());
}
void TypeLocWriter::VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) {
@@ -4481,16 +4469,21 @@
// General Serialization Routines
//===--===//
-/// Emit the list of attributes to the specified record.
-void ASTRecordWriter::AddAttributes(ArrayRef Attrs) {
+void ASTRecordWriter::AddAttr(const Attr *A) {
auto = *this;
- Record.push_back(Attrs.size());
- for (const auto *A : Attrs) {
-Record.push_back(A->getKind()); // FIXME: stable encoding, target attrs
-Record.AddSourceRange(A->getRange());
+ if (!A)
+return Record.push_back(0);
+ Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
+ Record.AddSourceRange(A->getRange());
#include "clang/Serialization/AttrPCHWrite.inc"
- }
+}
+
+/// Emit the list of attributes to the specified record.
+void ASTRecordWriter::AddAttributes(ArrayRef Attrs) {
+ push_back(Attrs.size());
+ for (const auto *A : Attrs)
+AddAttr(A);
}
void ASTWriter::AddToken(const Token , RecordDataImpl ) {
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2642,19 +2642,72 @@
// Attribute Reading
//===--===//
+namespace {
+class AttrReader {
+ ModuleFile *F;
+ ASTReader *Reader;
+ const ASTReader::RecordData
+ unsigned
+
+public:
+ AttrReader(ModuleFile , ASTReader ,
+ const ASTReader::RecordData , unsigned )
+ : F(), Reader(), Record(Record), Idx(Idx) {}
+
+ const uint64_t () { return Record[Idx++]; }
+
+ SourceRange readSourceRange() {
+return Reader->ReadSourceRange(*F, Record, Idx);
+ }
+
+ Expr *readExpr() { return Reader->ReadExpr(*F); }
+
+ std::string readString() {
+return Reader->ReadString(Record, Idx);
+ }
+
+ TypeSourceInfo *getTypeSourceInfo() {
+return Reader->GetTypeSourceInfo(*F, Record,
[PATCH] D50526: Model type attributes as regular Attrs
aaron.ballman added a comment.
Thank you for working on this -- the approach you've taken looks good, and I
mostly have little nits here and there. I think this is a great refactoring
overall though -- much needed!
Comment at: include/clang/AST/Type.h:1856
+
+ /// Was this type written with the special inert-in-MRC __unsafe_unretained
+ /// qualifier?
Typo: MRC should be ARC (typo was present in the original code).
Comment at: include/clang/Basic/Attr.td:1510
+ let Spellings = [Keyword<"__unsafe_unretained">];
+ let Documentation = [Undocumented];
+}
I don't suppose you can throw in some quick docs for this keyword? Or is this
not really user-facing? If it's not user-facing, perhaps it should have no
spellings and only ever be created implicitly?
Comment at: lib/AST/Type.cpp:1624
+ const Type *Cur = this;
+ while (auto *AT = Cur->getAs()) {
+if (AT->getAttrKind() == AK)
`const auto *`
Comment at: lib/AST/Type.cpp:3215
}
- llvm_unreachable("bad attributed type kind");
}
Probably need to keep this to prevent MSVC from barking about not all control
paths returning a value.
Comment at: lib/AST/Type.cpp:3653
+Optional
+Type::getNullability(const ASTContext ) const {
QualType type(this, 0);
Update the identifiers for coding standards since you're basically rewriting
the function?
Comment at: lib/AST/Type.cpp:3655
QualType type(this, 0);
- do {
+ while (auto *AT = type->getAs()) {
// Check whether this is an attributed type with nullability
`const auto *`
Comment at: lib/AST/TypeLoc.cpp:408
if (auto attributedLoc = getAs()) {
-if (attributedLoc.getAttrKind() == AttributedType::attr_nullable ||
-attributedLoc.getAttrKind() == AttributedType::attr_nonnull ||
-attributedLoc.getAttrKind() == AttributedType::attr_null_unspecified)
- return attributedLoc.getAttrNameLoc();
+auto *A = attributedLoc.getAttr();
+if (A && (isa(A) || isa(A) ||
Might as well go with `const Attr *` here, the `auto` gets us nothing.
Comment at: lib/Sema/SemaDecl.cpp:6024
// by applying it to the function type.
- if (ATL.getAttrKind() == AttributedType::attr_lifetimebound) {
+ if (auto *A = ATL.getAttrAs()) {
const auto *MD = dyn_cast(FD);
`const auto *`
Comment at: lib/Sema/SemaType.cpp:178-180
+// their TypeLocs makes it hard to correctly assign these. We use the
+// keep the attributes in creation order as an attempt to make them
+// line up properly.
"We use the keep the attributes" isn't grammatical. Should it be, "We keep the
attributes in creation order" instead?
Comment at: lib/Sema/SemaType.cpp:181
+// line up properly.
+using TypeAttr = std::pair;
+SmallVector TypeAttrs;
It's unfortunate to use the name `TypeAttr` here -- unqualified uses of the
type name, like below, makes it look like it might be the `TypeAttr` from Attr.h
Comment at: lib/Sema/SemaType.cpp:3884
+template
+static AttrT *createSimpleAttr(ASTContext , ParsedAttr ) {
Did clang-format do this? It seems like it's not formatted how I'd expect.
Comment at: lib/Sema/SemaType.cpp:6360
+ }
+}
+
MSVC may complain about not all control paths returning a value here.
Comment at: lib/Serialization/ASTReaderDecl.cpp:2695
+ Attr *New = nullptr;
+ auto Kind = (attr::Kind)(V - 1);
+ SourceRange Range = Record.readSourceRange();
This could use a comment to explain why the -1 is required. Also, do we have a
preference for C-style vs static_cast here?
Comment at: lib/Serialization/ASTReaderDecl.cpp:2707
void ASTReader::ReadAttributes(ASTRecordReader , AttrVec ) {
- for (unsigned i = 0, e = Record.readInt(); i != e; ++i) {
-Attr *New = nullptr;
-auto Kind = (attr::Kind)Record.readInt();
-SourceRange Range = Record.readSourceRange();
-ASTContext = getContext();
-
-#include "clang/Serialization/AttrPCHRead.inc"
-
-assert(New && "Unable to decode attribute?");
-Attrs.push_back(New);
- }
+ for (unsigned i = 0, e = Record.readInt(); i != e; ++i)
+Attrs.push_back(Record.readAttr());
Identifiers don't meet coding style rules.
Comment at: lib/Serialization/ASTWriter.cpp:4485-4486
+ push_back(Attrs.size());
+ for (const auto *A : Attrs)
+AddAttr(A);
}
`llvm::for_each(Attrs, AddAttr);` ?
Repository:
rC Clang
https://reviews.llvm.org/D50526
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[PATCH] D50526: Model type attributes as regular Attrs
rsmith created this revision.
rsmith added a reviewer: aaron.ballman.
rsmith added a project: clang.
Specifically, `AttributedType` now tracks a regular `attr::Kind` rather than
having its own parallel `Kind` enumeration, and `AttributedTypeLoc` now holds
an `Attr*` instead of holding an ad-hoc collection of fields mirroring those
that would be present in the corresponding `Attr` subclass.
This aims to simplify and unify the modeling of attributes, both to make
tooling simpler and to avoid code duplication for attributes that can be both
type and declaration attributes.
Repository:
rC Clang
https://reviews.llvm.org/D50526
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Attr.h
include/clang/AST/Type.h
include/clang/AST/TypeLoc.h
include/clang/Basic/Attr.td
include/clang/Sema/Sema.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/ASTWriter.h
lib/ARCMigrate/TransGCAttrs.cpp
lib/ARCMigrate/Transforms.cpp
lib/AST/ASTContext.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/AST/TypePrinter.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaInit.cpp
lib/Sema/SemaObjCProperty.cpp
lib/Sema/SemaType.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
lib/StaticAnalyzer/Core/CheckerHelpers.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2470,8 +2470,10 @@
static const AttrClassDescriptor AttrClassDescriptors[] = {
{ "ATTR", "Attr" },
+ { "TYPE_ATTR", "TypeAttr" },
{ "STMT_ATTR", "StmtAttr" },
{ "INHERITABLE_ATTR", "InheritableAttr" },
+ { "DECL_OR_TYPE_ATTR", "DeclOrTypeAttr" },
{ "INHERITABLE_PARAM_ATTR", "InheritableParamAttr" },
{ "PARAMETER_ABI_ATTR", "ParameterABIAttr" }
};
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -103,9 +103,9 @@
const auto *AttrType = Type->getAs();
if (!AttrType)
return Nullability::Unspecified;
- if (AttrType->getAttrKind() == AttributedType::attr_nullable)
+ if (AttrType->getAttrKind() == attr::TypeNullable)
return Nullability::Nullable;
- else if (AttrType->getAttrKind() == AttributedType::attr_nonnull)
+ else if (AttrType->getAttrKind() == attr::TypeNonNull)
return Nullability::Nonnull;
return Nullability::Unspecified;
}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -770,19 +770,7 @@
}
void TypeLocWriter::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
- Record.AddSourceLocation(TL.getAttrNameLoc());
- if (TL.hasAttrOperand()) {
-SourceRange range = TL.getAttrOperandParensRange();
-Record.AddSourceLocation(range.getBegin());
-Record.AddSourceLocation(range.getEnd());
- }
- if (TL.hasAttrExprOperand()) {
-Expr *operand = TL.getAttrExprOperand();
-Record.push_back(operand ? 1 : 0);
-if (operand) Record.AddStmt(operand);
- } else if (TL.hasAttrEnumOperand()) {
-Record.AddSourceLocation(TL.getAttrEnumOperandLoc());
- }
+ Record.AddAttr(TL.getAttr());
}
void TypeLocWriter::VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) {
@@ -4481,16 +4469,21 @@
// General Serialization Routines
//===--===//
-/// Emit the list of attributes to the specified record.
-void ASTRecordWriter::AddAttributes(ArrayRef Attrs) {
+void ASTRecordWriter::AddAttr(const Attr *A) {
auto = *this;
- Record.push_back(Attrs.size());
- for (const auto *A : Attrs) {
-Record.push_back(A->getKind()); // FIXME: stable encoding, target attrs
-Record.AddSourceRange(A->getRange());
+ if (!A)
+return Record.push_back(0);
+ Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
+ Record.AddSourceRange(A->getRange());
#include "clang/Serialization/AttrPCHWrite.inc"
- }
+}
+
+/// Emit the list of attributes to the specified record.
+void ASTRecordWriter::AddAttributes(ArrayRef Attrs) {
+ push_back(Attrs.size());
+ for (const auto *A : Attrs)
+AddAttr(A);
}
void ASTWriter::AddToken(const Token , RecordDataImpl ) {
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2642,19 +2642,70 @@
// Attribute Reading
//===--===//
+namespace {
+class AttrReader {
+ ModuleFile *F;
+ ASTReader *Reader;
+ const ASTReader::RecordData
+ unsigned
+
+public:
+
