Meinersbur marked an inline comment as done.
Meinersbur added inline comments.
================
Comment at: test/Sema/attr-ownership.c:22
void f15(int, int)
- __attribute__((ownership_returns(foo, 1))) // expected-note {{declared with
index 1 here}}
- __attribute__((ownership_returns(foo, 2))); // expected-error
{{'ownership_returns' attribute index does not match; here it is 2}}
+ __attribute__((ownership_returns(foo, 1))) // expected-error
{{'ownership_returns' attribute index does not match; here it is 1}}
+ __attribute__((ownership_returns(foo, 2))); // expected-note {{declared with
index 2 here}}
----------------
erichkeane wrote:
> This seems wrong to me, the 2nd one should be the error condition, right?
This is the result of how attributes are combined. There are the two
possibilities
```
void f15(int, int) __attribute__((ownership_returns(foo, 1)));
void f15(int, int) __attribute__((ownership_returns(foo, 2)));
```
and
```
void f15(int, int) __attribute__((ownership_returns(foo, 1)))
__attribute__((ownership_returns(foo, 2)));
```
The error diagnosis seem to have been written with the first case in mind and
emits in the order there. In the second case attributes merged in the other
order (which is the naively correct order, see
https://reviews.llvm.org/D48100#1142865), resulting diagnosis to be emitted in
the reverse-textual order. There is no consistency in which SourceLocation is
the first and which one is the second, and many diagnoses are already not
printed in textual order.
I cannot change this without massively reworking how attributes are processed
in clang.
================
Comment at: test/Sema/attr-print.c:25
// TODO: the Type Printer has no way to specify the order to print attributes
// in, and so it currently always prints them in reverse order. Fix this.
----------------
erichkeane wrote:
> This TODO doesn't apply, right? Or is at least wrong...
Correct, I missed this todo.
================
Comment at: test/Sema/attr-visibility.c:18
-void test6() __attribute__((visibility("hidden"), // expected-note {{previous
attribute is here}}
- visibility("default"))); // expected-error
{{visibility does not match previous declaration}}
+void test6() __attribute__((visibility("default"), // expected-error
{{visibility does not match previous declaration}}
+ visibility("hidden"))); // expected-note
{{previous attribute is here}}
----------------
erichkeane wrote:
> This order issue is likely to appear a couple of times I suspect.
See my previous reply and https://reviews.llvm.org/D48100#1142865
================
Comment at: test/SemaOpenCL/address-spaces.cl:64
typedef __private int private_int_t;
- __local __private int var1; // expected-error {{multiple address spaces
specified for type}}
- __local __private int *var2; // expected-error {{multiple address spaces
specified for type}}
+ __private __local int var1; // expected-error {{multiple address spaces
specified for type}}
+ __private __local int *var2; // expected-error {{multiple address spaces
specified for type}}
----------------
erichkeane wrote:
> These changes have me concerned... The error message isn't specific, but we
> have to change the order anyway?
An additional error is emitted with the reverse order (`non-kernel function
variable cannot be declared in local address space`; a result of which
attribute 'wins' in error resolution which is again related to which attribute
is already in the AttributeList). I'd say it is the responsibility diagnosis
code to emit the same set of messages independent of attribute order which I do
not try to fix here.
Repository:
rC Clang
https://reviews.llvm.org/D48100
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
