rogfer01 added a comment.
In https://reviews.llvm.org/D20561#662963, @royger wrote:
> In https://reviews.llvm.org/D20561#662959, @rogfer01 wrote:
>
> > We fixed all identified false positives in later patches to this one. So
> > maybe you want to check against trunk clang. If trunk still
rogfer01 added a comment.
In https://reviews.llvm.org/D20561#662922, @royger wrote:
>
Hi,
some targets do not support unaligned accesses so this warning is valuable in
terms of portability. In platforms where unaligned accesses are allowed, it may
happen that such accesses incur in a
rogfer01 added a comment.
In https://reviews.llvm.org/D20561#663069, @joerg wrote:
> It is not true that all false positives have been fixed, some of the more
> complex cases involving nested data structures are still open.
I could not find any in bugzilla after a quick search. So please feel
rogfer01 updated this revision to Diff 89172.
rogfer01 marked 2 inline comments as done.
rogfer01 added a comment.
Updated patch. Use TargetInfo::getCharWidth and remove unnecessary else.
https://reviews.llvm.org/D30166
Files:
include/clang/AST/ASTContext.h
lib/AST/ASTContext.cpp
rogfer01 created this revision.
See related proposal in
http://lists.llvm.org/pipermail/cfe-dev/2017-February/052739.html
This patch honors the unaligned type qualifier (currently available through he
keyword __unaligned and -fms-extensions) in CodeGen. In the current form the
patch affects
rogfer01 added a comment.
Sorry for the noise. I was expecting some more formal approval in the usual
form of "Ready to land" but if you think it is OK I will commit it shortly.
Regards.
https://reviews.llvm.org/D29986
___
cfe-commits mailing
This revision was automatically updated to reflect the committed changes.
Closed by commit rL296099: Fix crash when an incorrect redeclaration only
differs in __unaligned type… (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D29986?vs=88514=89617#toc
Repository:
rL
rogfer01 added a comment.
Ping?
https://reviews.llvm.org/D29986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 created this revision.
This patch fixes an assertion that is hit when a redeclaration with differing
types only differs in the unaligned type-qualifier.
https://reviews.llvm.org/D29986
Files:
lib/AST/ASTContext.cpp
test/Sema/unaligned-qualifier.c
Index:
rogfer01 added inline comments.
Comment at: lib/Driver/Tools.cpp:5940
+ options::OPT_fno_always_use_bss, false))
+CmdArgs.push_back("-always-use-bss");
+
I think you mean `"-falways-use-bss"`.
Please add a test that checks that the driver
rogfer01 updated this revision to Diff 89989.
rogfer01 added a comment.
Avoid altering results of TypeInfo due to the __unaligned qualifier. Instead
override the natural alignment to 1 if the type is __unaligned-qualified.
https://reviews.llvm.org/D30166
Files:
lib/AST/ASTContext.cpp
rogfer01 added a comment.
Thanks @javed.absar
This LGTM now.
https://reviews.llvm.org/D30326
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 added a comment.
Ping? :-)
Kind regards
https://reviews.llvm.org/D30166
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 created this revision.
rogfer01 added reviewers: EricWF, mclow.lists, rmaprath.
rogfer01 added a subscriber: cfe-commits.
The destructor of std::promise needs to construct a std::future_error
exception so it calls std::make_exception_ptr. But under
libcpp-no-exceptions this will trigger
rogfer01 added a comment.
Ping? :)
https://reviews.llvm.org/D27614
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 added inline comments.
Comment at: test/std/experimental/optional/optional.specalg/swap.pass.cpp:225
}
+#ifndef TEST_HAS_NO_EXCEPTIONS
{
mclow.lists wrote:
> Why is this here, and not before line L#236?
Yep, I'm wrong here. Probably a fallout
rogfer01 updated this revision to Diff 79765.
rogfer01 added a comment.
Protect the right set of tests.
https://reviews.llvm.org/D27254
Files:
test/std/experimental/optional/optional.specalg/swap.pass.cpp
Index: test/std/experimental/optional/optional.specalg/swap.pass.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288379: Protect optional test under libcpp-no-exceptions
(authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27254?vs=79765=79932#toc
Repository:
rL LLVM
rogfer01 created this revision.
rogfer01 added reviewers: EricWF, mclow.lists, rmaprath.
rogfer01 added a subscriber: cfe-commits.
Under libcpp-no-exceptions, noexcept is trivially true. Some tests expect in
the usual setting to return false. Wrap these tests in a convenience macro.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288383: Protect sequences test under libcpp-no-exceptions
(authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27252?vs=79717=79934#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288378: Protect std::ostream::sentry test under
libcpp-no-exceptions (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27255?vs=79720=79931#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288382: Protect futures test under libcpp-no-exceptions
(authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27253?vs=79718=79933#toc
Repository:
rL LLVM
rogfer01 created this revision.
rogfer01 added reviewers: mclow.lists, EricWF, rmaprath.
rogfer01 added a subscriber: cfe-commits.
Skip test that throws an exception.
https://reviews.llvm.org/D27255
Files:
test/std/input.output/iostream.format/output.streams/ostream_sentry/destruct.pass.cpp
rogfer01 created this revision.
rogfer01 added reviewers: mclow.lists, EricWF, rmaprath.
rogfer01 added a subscriber: cfe-commits.
Replace throw with TEST_THROW and protect tests that do throw. Also add missing
assert(false).
https://reviews.llvm.org/D27252
Files:
rogfer01 created this revision.
rogfer01 added reviewers: mclow.lists, EricWF, rmaprath.
rogfer01 added a subscriber: cfe-commits.
Skip tests that expect an exception be thrown.
https://reviews.llvm.org/D27253
Files:
test/std/thread/futures/futures.promise/move_assign.pass.cpp
Index:
rogfer01 created this revision.
rogfer01 added reviewers: mclow.lists, EricWF, rmaprath.
rogfer01 added a subscriber: cfe-commits.
Replace throw with TEST_THROW and skip tests that throw exceptions
https://reviews.llvm.org/D27254
Files:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288158: Protect std::string tests under libcpp-no-exceptions
(authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D26612?vs=78012=79583#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288165: Protect std::array tests under noexceptions
(authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27095?vs=79589=79591#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288157: Protect std::{,unordered_}map tests under
noexceptions (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27093?vs=79212=79582#toc
Repository:
rL LLVM
rogfer01 updated this revision to Diff 79589.
rogfer01 added a comment.
Add missing assertions in the original test.
https://reviews.llvm.org/D27095
Files:
test/std/containers/sequences/array/at.pass.cpp
Index: test/std/containers/sequences/array/at.pass.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288155: Protect test for dynarray under libcpp-no-exceptions
(authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D26611?vs=77809=79580#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288156: Protect locale tests under noexceptions (authored by
rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27096?vs=79219=79581#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288660: Handle tests for noexcept that expect a false value
(authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27310?vs=80238=80242#toc
Repository:
rL LLVM
rogfer01 updated this revision to Diff 80238.
rogfer01 added a comment.
Do not use constexpr.
https://reviews.llvm.org/D27310
Files:
test/libcxx/strings/iterators.exceptions.pass.cpp
Index: test/libcxx/strings/iterators.exceptions.pass.cpp
rogfer01 updated this revision to Diff 80235.
rogfer01 added a comment.
- Rewrite the adjusted tests in the way suggested by @EricWF
- Also check that this test works in a GCC-built libcxx
https://reviews.llvm.org/D27310
Files:
test/libcxx/strings/iterators.exceptions.pass.cpp
Index:
rogfer01 added inline comments.
Comment at: test/libcxx/strings/iterators.exceptions.pass.cpp:29
+#ifndef TEST_HAS_NO_EXCEPTIONS
+constexpr bool expected = false;
+#else
EricWF wrote:
> We can't use constexpr because these tests run in C++03. However `const bool
rogfer01 updated this revision to Diff 80081.
rogfer01 added a comment.
Drop attribute packed of a class when one of its fields is a non-packed non-pod
for compatibility with GCC.
https://reviews.llvm.org/D23325
Files:
include/clang/AST/Decl.h
include/clang/AST/Expr.h
This revision was automatically updated to reflect the committed changes.
Closed by commit rL291550: Mark tests as unsupported under libcpp-no-exceptions
(authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D27614?vs=80877=83782#toc
Repository:
rL LLVM
rogfer01 updated this revision to Diff 81955.
rogfer01 added a comment.
Rebase to current trunk
https://reviews.llvm.org/D23325
Files:
include/clang/AST/Decl.h
include/clang/AST/Expr.h
include/clang/AST/Stmt.h
include/clang/Basic/DiagnosticSemaKinds.td
rogfer01 updated this revision to Diff 82114.
rogfer01 added a comment.
Minor issues addressed
https://reviews.llvm.org/D23325
Files:
include/clang/AST/Decl.h
include/clang/AST/Expr.h
include/clang/AST/Stmt.h
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/Specifiers.h
rogfer01 added inline comments.
Comment at: lib/Driver/ToolChains/Clang.cpp:1882
+TakeNextArg = true;
}
} else {
I wonder if you should `break;` here if validation fails like the original code
did?
https://reviews.llvm.org/D31496
rogfer01 added inline comments.
Comment at: lib/Driver/ToolChains/Clang.cpp:1882
+TakeNextArg = true;
}
} else {
salari01 wrote:
> rogfer01 wrote:
> > I wonder if you should `break;` here if validation fails like the original
> >
rogfer01 created this revision.
Under `-fms-extensions` `__unaligned` is a type-qualifier that can be applied
to a non-static member function declaration.
This causes an assertion when mangling the name under Itanium, where that
qualifier is not mangled.
This patch justs makes the minimal
This revision was automatically updated to reflect the committed changes.
Closed by commit rL300686: Avoid assert when a non-static member function is
qualified with __unaligned (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D31976?vs=94962=95723#toc
Repository:
rogfer01 created this revision.
Given that we have already explicitly stated in the qualifier that the
expression is `__unaligned`,
it makes little sense to diagnose that the address of the packed member may not
be aligned.
https://reviews.llvm.org/D30884
Files:
lib/Sema/SemaChecking.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297620: When diagnosing taking address of packed members
skip __unaligned-qualified… (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D30884?vs=91531=91548#toc
Repository:
rogfer01 added a comment.
Thanks for the review @aaron.ballman!
https://reviews.llvm.org/D30884
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 created this revision.
This is the clang side of the RFC in
http://lists.llvm.org/pipermail/cfe-dev/2017-February/052549.html
Note that in contrast to the original suggestion `-fsource-asm` here we use the
preferred `-fverbose-asm`. Basically explicitly saying `-fverbose-asm` in the
rogfer01 added a comment.
> A somewhat obvious comment: no chance for this to be accepted without LIT
> tests. I understand you have your doubts on the best approach to testing --
> and it's OK to ask either here or on llvm-dev -- but tests should be added
> nevertheless.
Sure. I wasn't
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297276: Honor __unaligned in codegen for declarations and
expressions (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D30166?vs=89989=91005#toc
Repository:
rL LLVM
rogfer01 added a comment.
Ping? :-)
Kind regards
https://reviews.llvm.org/D30166
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 added a comment.
`clang::Sema::IsOverload` explicitly forbids the `__restrict` case for the
qualifier of non-static member functions. I assume `__unaligned` is not
forbidden because the MSVC ABI does encode this qualifier while the Itanium ABI
currently does not.
This patch just
rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.
This LGTM. Thanks for the refactoring and the fix.
https://reviews.llvm.org/D34686
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rogfer01 added a comment.
I agree with the spirit of the change but I think you want to use `AArch64::`
enumerators instead of those in `ARM::`
Comment at: lib/Basic/Targets.cpp:6190
unsigned HasFullFP16;
+ unsigned ArchKind;
There is an
rogfer01 added a comment.
Hi @ogoffart thanks for fixing this.
I suggest some minor change for the check, other than that this looks good to
me.
Comment at: lib/Sema/SemaChecking.cpp:12099
BaseType = BaseType->getPointeeType();
RecordDecl *RD =
rogfer01 added inline comments.
Comment at: lib/CodeGen/CodeGenModule.cpp:4222
+ std::string line;
+ while (std::getline(iss, s, ';')) {
+if ((s.length() + line.length()) < 80) {
I wonder if this also works if the original compiler invokation included a
rogfer01 added a comment.
Given that we are already considering extensions, maybe you want to add a test
for compound statements in expressions.
void foo(void)
{
({ bar(); });
int (*p)() = bar; /* expected-error {{use of undeclared identifier 'bar'}}
*/
}
This already works with
rogfer01 created this revision.
`__unaligned` is not currently mangled in any way and as such is not a relevant
qualifier for substitutions. This may cause that, when it is the only
qualifier, the substitution is added twice which triggers an assertion.
The simplest thing to do is to remove
rogfer01 updated this revision to Diff 100670.
rogfer01 added a comment.
Thanks @aaron.ballman for the review.
I have extended the test with your suggestions.
https://reviews.llvm.org/D33398
Files:
lib/AST/ItaniumMangle.cpp
test/CodeGenCXX/pr33080.cpp
rogfer01 added a comment.
This clang change looks sensible to me. Can you add documentation in
`LanguageExtensions.rst` describing this new pragma and its semantics? Thanks!
https://reviews.llvm.org/D33412
___
cfe-commits mailing list
rogfer01 added a comment.
I wonder what should we do in C99 for cases like
void foo(void) {
{
extern void g(int(*a)[call_to_undeclared()]);
int (*p_ok)() = call_to_undeclared;
}
int (*p_err)() = call_to_undeclared;
}
I understand that the scope `S` in this case
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304523: Mangle __unaligned in Itanium ABI (authored by
rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D33398?vs=100974=101171#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33398
rogfer01 updated this revision to Diff 100868.
rogfer01 added a comment.
Fix formatting of tests.
Thanks for the review @aaron.ballman !
https://reviews.llvm.org/D33398
Files:
lib/AST/ItaniumMangle.cpp
test/CodeGenCXX/pr33080.cpp
test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
rogfer01 updated this revision to Diff 100974.
rogfer01 added a comment.
Changelog:
- Fix formatting.
- Emit `__unaligned` in the right order (as defined by the Itanium ABI) when
there are Objective-C++ ARC vendor qualifiers.
- New test for `__unaligned` and ARC's `__weak`, `__strong` and
rogfer01 added inline comments.
Comment at: lib/AST/ItaniumMangle.cpp:2210
+ if (Quals.hasUnaligned())
+ mangleVendorQualifier("__unaligned");
+
rsmith wrote:
> Too much indentation here. Also, the ABI requires the "unordered" vendor
> qualifiers to be
rogfer01 added a comment.
After the docs update this looks good to me.
@rnk @jroelofs what do you think? Thanks!
https://reviews.llvm.org/D33412
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rogfer01 added inline comments.
Comment at: include/clang/AST/Type.h:1669
bool isHalfType() const; // OpenCL 6.1.1.1, NEON (IEEE 754-2008 half)
+ bool isFloat16Type() const; // FIXME
bool isRealType() const; // C99 6.2.5p17 (real floating + integer)
rogfer01 added a comment.
Thanks Momchil, this looks sensible to me.
What do you think @aaron.ballman @rsmith ?
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rogfer01 added inline comments.
Comment at: include/clang-c/Index.h:3015
CXType_Half = 31,
+ CXType_Float16 = 30,
CXType_FirstBuiltin = CXType_Void,
This enumerator is the same as `CXType_Float128` above, is that intended?
rogfer01 added a comment.
I think what happens is that the implicit declaration is done in the prototype
scope which is not visible later.
Doing the same as GCC in the handling of this extension is probably the best:
note that in C90 a function call should not happen inside an array expression
rogfer01 accepted this revision.
rogfer01 added a subscriber: rsmith.
rogfer01 added a comment.
This revision is now accepted and ready to land.
This LGTM, but wait a couple of days before comitting in case @rsmith or
@scanon (or others!) have further comments.
https://reviews.llvm.org/D34695
rogfer01 created this revision.
https://reviews.llvm.org/D38628
Files:
test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
Index: test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
===
---
rogfer01 updated this revision to Diff 118005.
rogfer01 retitled this revision from "Mark test as unsupported in C++98 as
well" to "Mark tests as unsupported in C++98 as well".
rogfer01 added a comment.
ChangeLog:
- I wanted to mark two tests but forgot to add one in the previous change
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315278: Remove unneeded typename from test (authored by
rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D38628?vs=118183=118321#toc
Repository:
rL LLVM
https://reviews.llvm.org/D38628
rogfer01 added a comment.
Thanks @chill , @mclow.lists and @EricWF !
https://reviews.llvm.org/D38628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 updated this revision to Diff 118183.
rogfer01 added a comment.
- Remove another instance of this issue
https://reviews.llvm.org/D38628
Files:
test/std/utilities/utility/pairs/pair.astuple/tuple_element.fail.cpp
rogfer01 added a comment.
Friendly ping :)
https://reviews.llvm.org/D38452
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315570: Mark test as a long-test (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D38452?vs=117317=118753#toc
Repository:
rL LLVM
https://reviews.llvm.org/D38452
Files:
rogfer01 added inline comments.
Comment at: lib/Headers/float.h:137
+#ifdef __STDC_WANT_IEC_60559_TYPES_EXT__
+# define FLT16_MANT_DIG __FLT16_MANT_DIG__
My understanding is that, given that we support TS18661-2 by default, this
macro should be predefined
rogfer01 added inline comments.
Comment at: lib/Headers/float.h:137
+#ifdef __STDC_WANT_IEC_60559_TYPES_EXT__
+# define FLT16_MANT_DIG __FLT16_MANT_DIG__
scanon wrote:
> rogfer01 wrote:
> > My understanding is that, given that we support TS18661-2 by
rogfer01 created this revision.
https://reviews.llvm.org/D38362
Files:
test/std/re/re.alg/re.alg.match/exponential.pass.cpp
Index: test/std/re/re.alg/re.alg.match/exponential.pass.cpp
===
---
rogfer01 created this revision.
This test creates a string of 2GiB which may make it too slow to run in a
simulator.
https://reviews.llvm.org/D38452
Files:
test/std/input.output/stream.buffers/streambuf/streambuf.protected/streambuf.put.area/pbump2gig.pass.cpp
Index:
rogfer01 added a comment.
There are a few tests checking the precise interaction between `__fp16` and
`_Float16` values but I don't see tests interacting `_Float16` and the other
standard floating types `float`, `double` and `long double`.
Do you reckon it is worth adding them as well?
rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.
Thanks @SjoerdMeijer
This LGTM now. Wait a couple of days in case @rsmith has more comments.
https://reviews.llvm.org/D33719
___
rogfer01 added inline comments.
Comment at: lib/Sema/SemaTemplate.cpp:7021
+ // C++ [temp]p6:
+ // A template, a template explicit specialization, and a class template
+ // partial specialization shall not have C linkage.
Can you add a test for a partial
rogfer01 added inline comments.
Comment at: lib/Sema/SemaTemplate.cpp:7021
+ // C++ [temp]p6:
+ // A template, a template explicit specialization, and a class template
+ // partial specialization shall not have C linkage.
miyuki wrote:
> rogfer01 wrote:
>
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317210: Mark tests as unsupported in C++98 as well (authored
by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D38362?vs=118005=121310#toc
Repository:
rL LLVM
rogfer01 added a comment.
Thank you @mclow.lists !
Repository:
rL LLVM
https://reviews.llvm.org/D38362
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL320017: Ignore pointers to incomplete types when diagnosing
misaligned addresses (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D40895?vs=125718=125901#toc
Repository:
rL
This revision was automatically updated to reflect the committed changes.
Closed by commit rL320363: [libcxx] Define istream_iterator equality comparison
operators out-of-line (authored by rogfer01).
Changed prior to commit:
https://reviews.llvm.org/D40415?vs=124155=126349#toc
Repository:
rogfer01 added a comment.
Hi @miyuki I can commit it.
https://reviews.llvm.org/D40415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 created this revision.
This is a fix for PR35509 in which we crash because we attempt to compute the
alignment of an incomplete type.
https://reviews.llvm.org/D40895
Files:
lib/Sema/SemaChecking.cpp
test/SemaCXX/address-packed.cpp
Index: test/SemaCXX/address-packed.cpp
rogfer01 added a comment.
@rjmccall because we do not want to impact the clients of `ABIArgInfo` I
thought of two possible approaches
1. extend `CGFunctionInfo` with a third trailing array (now it has two), with
as many elements as `ABIArgInfo` (call it `ABIArgExtraInfo`) then during the
rogfer01 updated this revision to Diff 153070.
rogfer01 added a comment.
ChangeLog:
- Use a `ConstantDataArray` instead of a struct of types.
- Use `LLVM_IS_TRIVIALLY_COPYABLE`
https://reviews.llvm.org/D48589
Files:
include/clang/CodeGen/CGFunctionInfo.h
lib/CodeGen/CGCall.cpp
Index:
rogfer01 added inline comments.
Comment at: include/clang/CodeGen/CGFunctionInfo.h:475
+static_assert(std::is_trivially_copyable::value,
+"ABIArgInfo must be trivially copyable as it is embedded as trailing "
I think this is the right trait here. I spent
rogfer01 created this revision.
rogfer01 added a reviewer: rjmccall.
This is WIP and it is motivated by the suggestions in
http://lists.llvm.org/pipermail/cfe-dev/2018-June/058263.html
First attempt, piggybacking the extend information in a structure where the
bit-width of the integer
rogfer01 added inline comments.
Comment at: include/clang/CodeGen/CGFunctionInfo.h:90
union {
-unsigned DirectOffset; // isDirect() || isExtend()
-unsigned IndirectAlign;// isIndirect()
-unsigned AllocaFieldIndex; // isInAlloca()
+llvm::StructType
rogfer01 added a comment.
I understand you're fixing the narrowing check from "unscoped enum/integer
type" → float.
But you have also extended some tests which are "unscoped enum" → integer type
(lines 153, 170 and 173). So I presume they were already handled correctly
before your patch. Can
rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.
This LGTM. Thanks for fixing this.
Wait a couple of days before committing in case there are other comments.
https://reviews.llvm.org/D42969
rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.
Looks good to me now. Wait a couple of days before submitting it just in case
the other reviewers have more comments.
Do you plan to submit (in another change) the unscoped enum → integer
1 - 100 of 208 matches
Mail list logo
