Anastasia marked an inline comment as done.
Anastasia added inline comments.
================
Comment at: lib/Parse/ParseDecl.cpp:6162
+ }
+ }
+
----------------
rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > This is enforcing a restriction that users write `const __private`, which
> > > seems unreasonable. It looks like `ParseTypeQualifierList` takes a flag
> > > saying not to parse attributes; try adding a new option to that enum
> > > allowing address-space attributes.
> > >
> > > Collecting the attributes on the type-qualifiers `DeclSpec` rather than
> > > adding them as function attributes seems correct.
> > Do you mean `ParseTypeQualifierListOpt`? That already parses the address
> > spaces unconditionally. The problem is however that we are using local
> > `DeclSpec` - `DS` variable here during the function qualifiers parsing
> > because the `DeclSpec` member of the `Declarator` corresponds to the return
> > type as far as I understand it. Therefore I am propagating missing address
> > space attributes from the local `DS` variable into `FnAttrs` to be used in
> > the function type info.
> >
> > With current patch both of the following two forms:
> > struct C {
> > void foo() __local const;
> > };
> > and
> > struct C {
> > void foo() const __local;
> > };
> > would be parsed correctly generating identical IR
> > declare void @_ZNU3AS3K1C3fooEv(%struct.C addrspace(3)*)
> >
> >
> >
> Oh, I see, sorry. Why filter on attributes at all, then? We should *only*
> be parsing qualifier attributes in that list.
>
> I actually think it would be reasonable to change
> `DeclaratorChunk::FunctionTypeInfo` to just store a `DeclSpec` for all the
> qualifiers; we're already duplicating an unfortunate amount of the logic from
> `DeclSpec`, like remembering `SourceLocation`s for all the qualifiers.
> You'll have to store it out-of-line, but we already store e.g. parameters out
> of line, so the machinery for allocating and destroying it already exists.
> Just make sure we don't actually allocate anything in the common case where
> there aren't any qualifiers (i.e. for C and non-member C++ functions).
>
> Also, I suspect that the use of `CXXThisScopeRAII` below this needs to be
> updated to pull *all* the qualifiers out of `DS`. Maybe Sema should have a
> function for that.
I have uploaded a separate patch for this:
https://reviews.llvm.org/D55948
I think I can't avoid storing `DeclSpec` for non-memeber functions in C++
because we still use them to give diagnostics about the qualifiers. For example:
test.cpp:1:12: error: non-member function cannot have 'const' qualifier
void foo() const;
As for `CXXThisScopeRAII`, we seem to be adding other qualifiers to the
`QualType` directly (while building it), so there seems to be no function to
collect them into `Qualifiers`. I can, however, add code to collect address
space qualifiers here. I guess we don't have any other missing qualifiers to
collect?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55850/new/
https://reviews.llvm.org/D55850
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
