ubsan added a comment.
The ABI breakage was already there, in the difference between GCC and Clang
- if one compiles against libc++ with gcc, it is not compatible with things
that are compiled with clang. I frankly think that being ABI compatible
with gcc (and correct according to the standard)
ldionne added subscribers: dexonsmith, ldionne.
ldionne added a comment.
I'm surprised that nobody discussed the potential for ABI breakage of this
change, which changes the result of the `alignof` operator. I'm trying to
evaluate what to do in libc++ as a result of this change (now
`std::align
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345419: PR26547: alignof should return ABI alignment, not
preferred alignment (authored by rsmith, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.
ubsan updated this revision to Diff 171247.
ubsan added a comment.
fix nits
Repository:
rC Clang
https://reviews.llvm.org/D53207
Files:
docs/ReleaseNotes.rst
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Ba
rsmith added a comment.
In https://reviews.llvm.org/D53207#1276062, @ubsan wrote:
> I don't actually know how to send this upstream, since it's been accepted...
> How should I proceed?
If you don't have an SVN account, you can ask someone who does to commit it for
you. (In this case, I'll do
ubsan added a comment.
I don't actually know how to send this upstream, since it's been accepted...
How should I proceed?
Repository:
rC Clang
https://reviews.llvm.org/D53207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
ubsan updated this revision to Diff 170994.
ubsan added a comment.
Add ABI breakage information and reflow
Repository:
rC Clang
https://reviews.llvm.org/D53207
Files:
docs/ReleaseNotes.rst
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Basic/Diagnostic
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Thanks! Can you also write something for the release notes
(docs/ReleaseNotes.rst) describing the ABI change?
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5478
de
ubsan updated this revision to Diff 170814.
ubsan added a comment.
Finish doing the thing! all tests are passing, at least on the revision I'm
running on :)
@rsmith please check the thing!
Repository:
rC Clang
https://reviews.llvm.org/D53207
Files:
include/clang/AST/Stmt.h
include/clan
ubsan added a comment.
... Yup, that'd do it. Thanks Richard!
Repository:
rC Clang
https://reviews.llvm.org/D53207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added a comment.
In https://reviews.llvm.org/D53207#1273727, @ubsan wrote:
> when I don't specifically don't set the type trait kind of `__alignof` to
> `UETT_PreferredAlignOf`, and remove the code in `AlignOfType` to
> differentiate between `PreferredAlignOf` and `AlignOf`, all the test
ubsan added a comment.
> Is it possible you have LLVM and Clang checked out at different revisions?
> That's a common source of weird behavior, particularly in CodeGen tests.
Unfortunately no - when I don't specifically don't set the type trait kind of
`__alignof` to `UETT_PreferredAlignOf`, an
rsmith added a comment.
In https://reviews.llvm.org/D53207#1268535, @ubsan wrote:
> I'm having real difficulty with this - I get really odd errors in seemingly
> unrelated tests when I change things.
Is it possible you have LLVM and Clang checked out at different revisions?
That's a common so
ubsan added a comment.
I'm having real difficulty with this - I get really odd errors in seemingly
unrelated tests when I change things.
Repository:
rC Clang
https://reviews.llvm.org/D53207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rsmith added a comment.
Please extend the tests to check that the `-fclang-abi-compat=7` switch undoes
your change here.
One other comment, and otherwise this looks good. Will you need someone to
commit this for you?
Comment at: test/SemaCXX/align-x86.cpp:4-5
+
+#include
+#
ubsan updated this revision to Diff 169503.
ubsan added a comment.
add tests
fix nits
Repository:
rC Clang
https://reviews.llvm.org/D53207
Files:
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Basic/LangOptions.h
include/clang/Basic/TypeTraits.h
lib/AST/ASTDumper.cpp
lib/AS
rsmith added a comment.
Thanks, this generally looks good, but it needs tests. For `_Alignof`,
test/Sema/align-x86.c` would be a reasonable place for the test; for C++
`alignof`, I don't see a suitable existing test file, but you could add one to
test/SemaCXX, perhaps based on test/Sema/align-x
ubsan created this revision.
ubsan added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.
- Add `UETT_PreferredAlignOf` to account for the difference between `__alignof`
and `alignof`
- `AlignOfType` now returns ABI alignment instead of preferred alignment iff
clang-abi-compat > 7, an
This is a patch to fix bug 26547
(https://bugs.llvm.org/show_bug.cgi?id=26547)
This is my first patch, so please give feedback! This should be correct
though, and I've tested it on both Windows x86 (where it does nothing), and
Linux x86 (where it correctly returns alignof(double) = 4, __al
19 matches
Mail list logo
