[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
dexonsmith added inline comments.
Comment at: src/cxa_demangle.cpp:3042-3051
+for (size_t k = k0; k < k1; ++k) {
+auto tmp = db.names[k].move_full();
+if (!tmp.empty())
+{
+if (!is_first_it)
+db.names[lambda_pos].first.append(", ");
+is_first_it = false;
compnerd wrote:
> I think that using a bit of algorithm here might be nice.
>
> std::ostringstream oss(db.names[lambda_pos]);
> std::copy_if(db.names[k0], db.names[k1],
> std::ostream_iterator(oss, ", "),
> [](const std::string ) -> bool { return !s.empty(); });
Introducing a dependency on `std::ostream` would be awkward. libc++abi.dylib
cannot link against libc++.dylib, and it would bloat libc++abi.dylib to use
custom traits to pull in a full copy.
https://reviews.llvm.org/D33368
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal
compnerd added inline comments.
Comment at: cmake/caches/BaremetalARM.cmake:1
+set(LLVM_TARGETS_TO_BUILD ARM CACHE STRING "")
+
jroelofs wrote:
> compnerd wrote:
> > Please rename this file to `BareMetalARMv6.cmake`. (I'm interested in the
> > suffix primarily).
> My plan is to eventually add multilibs to this configuration, so while that
> makes sense short term, I don't think it makes sense long term.
>
> With that in mind, do you still want me to rename it?
I think it makes sense longer term still. ARMv6 vs ARMv7. Even if you do
multilib, that wouldnt take care of that right?
Comment at: lib/Driver/ToolChains/BareMetal.cpp:68
+ SmallString<128> Dir(getDriver().ResourceDir);
+ llvm::sys::path::append(Dir, "lib", "baremetal");
+ return Dir.str();
jroelofs wrote:
> compnerd wrote:
> > Why not just the standard `arm` directory?
> There are a few differences between the stuff in the existing ones, and what
> is needed on baremetal. For example __enable_execute_stack, emutls, as well
> as anything else that assumes existence of pthreads support shouldn't be
> there.
Well, I think that "baremetal" here is a bad idea. How about using the android
approach? Use `clang_rt.builtins-arm-baremetal.a` ?
Comment at: lib/Driver/ToolChains/BareMetal.cpp:107-108
+ArgStringList ) const {
+ CmdArgs.push_back("-lc++");
+ CmdArgs.push_back("-lc++abi");
+ CmdArgs.push_back("-lunwind");
jroelofs wrote:
> jroelofs wrote:
> > compnerd wrote:
> > > I think that this is a bit extreme. We already have `-stdlib=libc++` and
> > > `-stdlib=libstdc++`. Why not just honor that?
> > I wasn't going for "support every possible thing out of the tin", instead
> > preferring incremental development :)
> Added support for `-stdlib=`.
>
> I made the assumption that `-stdlib=libstdc++` implies the user wants
> libsupc++ as their ABI library, though someone may want libstdc++ +
> libc++abi, or other combinations. If that's the case, we can add `-abilib=`,
> and likewise `-unwinder=`... but let's save that for another patch, on
> another day.
Yeah, switching between libc++/libc++abi and libstdc++/libsupc++ sounds
reasonable. Anything beyond that is something which doesnt work well within
the driver anyways. I should polish off my `-unwinder` patch.
Comment at: lib/Driver/ToolChains/BareMetal.h:42
+
+ const char *getDefaultLinker() const override { return "ld.lld"; }
+
jroelofs wrote:
> compnerd wrote:
> > I think that this really should be `ld` still, as that is the canonical
> > name for the linker.
> You mean `lld`?
>
> ```
> $ lld
> lld is a generic driver.
> Invoke ld.lld (Unix), ld (macOS) or lld-link (Windows) instead.
> ```
>
> Or are you saying: "make binutils ld the default, not llvm's lld"?
Im saying use the name "ld". ld is a symlink on most linux distributions these
days. ld -> ld.gold, ld.bfd, ld.lld
https://reviews.llvm.org/D33259
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)
faisalv closed this revision. faisalv added a comment. Committed here : https://reviews.llvm.org/rL303492 https://reviews.llvm.org/D31588 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.
faisalv closed this revision. faisalv added a comment. Closed by commit https://reviews.llvm.org/rL295279 https://reviews.llvm.org/D29748 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.
Comment at: src/cxa_demangle.cpp:3036
break;
-if (db.names.size() < 2)
+if (k1 <= k0)
return first;
I'm not sure how `k1` can be `<` than `k0`. Isn't this effectively always
going down the `==` path? If I'm just mistaken, then I think that this needs a
comment.
Comment at: src/cxa_demangle.cpp:3042-3051
+for (size_t k = k0; k < k1; ++k) {
+auto tmp = db.names[k].move_full();
+if (!tmp.empty())
+{
+if (!is_first_it)
+db.names[lambda_pos].first.append(", ");
+is_first_it = false;
I think that using a bit of algorithm here might be nice.
std::ostringstream oss(db.names[lambda_pos]);
std::copy_if(db.names[k0], db.names[k1],
std::ostream_iterator(oss, ", "),
[](const std::string ) -> bool { return !s.empty(); });
https://reviews.llvm.org/D33368
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33385: __cxa_demangle: Fix constructor cv qualifier handling
compnerd added a comment. Can you please ensure that you cross-port this into `llvm/lib/Demangle/ItaniumDemangle.cpp`? https://reviews.llvm.org/D33385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33082: Fix Libc++ build with MinGW64
martell added a comment. I want to give some context here to dispel the confusion of what is and isn't win32 api specific. First lets take `vasprintf` and `asprintf ` which are not implemented in msvcrt. In mingw-w64 we would just have posix implementations of both. They are hidden behind the guard `_GNU_SOURCE`, we don't need to declare them in `include/stdio.h` in libcxx but By default gcc doesn't pass this flag for the mingw target so we should add it in cmake within libc++. We should be able to guard mingw to stop it from picking up the implementations in `windows/support.cpp` Next example Example `mbsnrtowcs` and `wcsnrtombs` above This is technically a win32api specific implementation in msvc. The mingw-w64 implementation or lack there of would rely on the MSVC implementation. This is why a custom implementation lives in libc++ in `support.cpp` that is posix compliant. Unfortunately a posix implementation might not be accepted upstream into mingw-w64 because they need to maintain compatibility with msvc, even with some bugs eek. It generally comes down to if a bunch of projects are using it already and we should not disrupt them relying on msvc implementations/bugs. This is probably not the case for these two functions but there are many others already in the library that follow this guideline. A good read of this specific bug is here. http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130325/077071.html @smeenai suggestion of `_LIBCPP_MSVCRT_LIKE` makes a lot of sense here because mingw-w64 is only partially posixy, this leaves room for future possible targets like midipix which uses musl libc to ignore this section of code where mingw and msvc opt in. https://reviews.llvm.org/D33082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33363: [mips] Support `micromips` attribute
atanasyan updated this revision to Diff 99703.
atanasyan added a comment.
Thanks for review.
This changes:
- Allow simultaneous using of micromips and interrupt attributes.
- Fix wording of micromips attribute description.
Repository:
rL LLVM
https://reviews.llvm.org/D33363
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/CodeGen/TargetInfo.cpp
lib/Sema/SemaDeclAttr.cpp
test/CodeGen/micromips-attr.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-micromips.c
Index: test/Sema/attr-micromips.c
===
--- /dev/null
+++ test/Sema/attr-micromips.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple mips-linux-gnu -fsyntax-only -verify %s
+
+__attribute__((nomicromips(0))) void foo1(); // expected-error {{'nomicromips' attribute takes no arguments}}
+__attribute__((micromips(1))) void foo2();// expected-error {{'micromips' attribute takes no arguments}}
+
+__attribute((nomicromips)) int a; // expected-error {{attribute only applies to functions}}
+__attribute((micromips)) int b; // expected-error {{attribute only applies to functions}}
+
+__attribute__((micromips,mips16)) void foo5(); // expected-error {{'micromips' and 'mips16' attributes are not compatible}} \
+// expected-note {{conflicting attribute is here}}
+__attribute__((mips16,micromips)) void foo6(); // expected-error {{'mips16' and 'micromips' attributes are not compatible}} \
+// expected-note {{conflicting attribute is here}}
+
+__attribute((micromips)) void foo7();
+__attribute((nomicromips)) void foo8();
Index: test/Misc/pragma-attribute-supported-attributes-list.test
===
--- test/Misc/pragma-attribute-supported-attributes-list.test
+++ test/Misc/pragma-attribute-supported-attributes-list.test
@@ -2,7 +2,7 @@
// The number of supported attributes should never go down!
-// CHECK: #pragma clang attribute supports 60 attributes:
+// CHECK: #pragma clang attribute supports 62 attributes:
// CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
@@ -30,8 +30,10 @@
// CHECK-NEXT: IFunc (SubjectMatchRule_function)
// CHECK-NEXT: InternalLinkage (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
// CHECK-NEXT: LTOVisibilityPublic (SubjectMatchRule_record)
+// CHECK-NEXT: MicroMips (SubjectMatchRule_function)
// CHECK-NEXT: NoDebug (SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
// CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
+// CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
// CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
// CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
// CHECK-NEXT: NoSplitStack (SubjectMatchRule_function)
Index: test/CodeGen/micromips-attr.c
===
--- /dev/null
+++ test/CodeGen/micromips-attr.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+void __attribute__((micromips)) foo (void) {}
+
+// CHECK: define void @foo() [[MICROMIPS:#[0-9]+]]
+
+void __attribute__((nomicromips)) nofoo (void) {}
+
+// CHECK: define void @nofoo() [[NOMICROMIPS:#[0-9]+]]
+
+// CHECK: attributes [[MICROMIPS]] = { noinline nounwind {{.*}} "micromips" {{.*}} }
+// CHECK: attributes [[NOMICROMIPS]] = { noinline nounwind {{.*}} "nomicromips" {{.*}} }
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5935,12 +5935,18 @@
handleDLLAttr(S, D, Attr);
break;
case AttributeList::AT_Mips16:
-handleSimpleAttributeWithExclusions(S, D,
- Attr);
+handleSimpleAttributeWithExclusions(S, D, Attr);
break;
case AttributeList::AT_NoMips16:
handleSimpleAttribute(S, D, Attr);
break;
+ case AttributeList::AT_MicroMips:
+handleSimpleAttributeWithExclusions(S, D, Attr);
+break;
+ case AttributeList::AT_NoMicroMips:
+handleSimpleAttribute(S, D, Attr);
+break;
case AttributeList::AT_AMDGPUFlatWorkGroupSize:
handleAMDGPUFlatWorkGroupSizeAttr(S, D, Attr);
break;
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -6557,6
[PATCH] D33094: [ASTMatchers] Add clang-query support for equals matcher
aaron.ballman added inline comments.
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3846
+ CXXBoolLiteralExpr,
+ IntegerLiteral),
+ unsigned, Value, 1) {
Is there a reason to not allow the equals matcher to do something like
`floatingLiteral(equals(1))`? Sure, the user could always write `1.0`, but it
seems somewhat hostile to require it.
Comment at: unittests/ASTMatchers/Dynamic/RegistryTest.cpp:534
+ EXPECT_TRUE(matches("double x = 12e-1;", DoubleStmt));
+ EXPECT_FALSE(matches("double x = 1.23;", DoubleStmt));
+
Can you add tests for floating literals with suffixes (f, l)?
Comment at: unittests/ASTMatchers/Dynamic/RegistryTest.cpp:545
+ .getTypedMatcher();
+ EXPECT_TRUE(matches("int x = 'x';", CharStmt));
+ EXPECT_FALSE(matches("int x = 120;", CharStmt));
Can you add some tests involving the other character literal types (L, u, U,
u8)?
https://reviews.llvm.org/D33094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30170: Function definition may have uninstantiated body
sepavloff updated this revision to Diff 99698.
sepavloff added a comment.
Made the patch a bit more compact. NFC.
https://reviews.llvm.org/D30170
Files:
include/clang/AST/Decl.h
lib/AST/Decl.cpp
lib/Sema/SemaDecl.cpp
test/SemaCXX/cxx0x-cursory-default-delete.cpp
test/SemaCXX/friend2.cpp
Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -101,6 +101,34 @@
friend void func_12(int x = 0); // expected-error{{friend declaration specifying a default argument must be the only declaration}}
};
+// Friend function with uninstantiated body is still a definition.
+
+template struct C20 {
+ friend void func_20() {} // expected-note{{previous definition is here}}
+};
+C20 c20i;
+void func_20() {} // expected-error{{redefinition of 'func_20'}}
+
+template struct C21a {
+ friend void func_21() {} // expected-note{{previous definition is here}}
+};
+template struct C21b {
+ friend void func_21() {} // expected-error{{redefinition of 'func_21'}}
+};
+C21a c21ai;
+C21b c21bi; // expected-note{{in instantiation of template class 'C21b' requested here}}
+
+template struct C22a {
+ friend void func_22() {} // expected-note{{previous definition is here}}
+};
+template struct C22b {
+ friend void func_22();
+};
+C22a c22ai;
+C22b c22bi;
+void func_22() {} // expected-error{{redefinition of 'func_22'}}
+
+
namespace pr22307 {
Index: test/SemaCXX/cxx0x-cursory-default-delete.cpp
===
--- test/SemaCXX/cxx0x-cursory-default-delete.cpp
+++ test/SemaCXX/cxx0x-cursory-default-delete.cpp
@@ -136,13 +136,13 @@
};
struct DefaultDelete {
- DefaultDelete() = default; // expected-note {{previous declaration is here}}
+ DefaultDelete() = default; // expected-note {{previous definition is here}}
DefaultDelete() = delete; // expected-error {{constructor cannot be redeclared}}
- ~DefaultDelete() = default; // expected-note {{previous declaration is here}}
+ ~DefaultDelete() = default; // expected-note {{previous definition is here}}
~DefaultDelete() = delete; // expected-error {{destructor cannot be redeclared}}
- DefaultDelete =(const DefaultDelete &) = default; // expected-note {{previous declaration is here}}
+ DefaultDelete =(const DefaultDelete &) = default; // expected-note {{previous definition is here}}
DefaultDelete =(const DefaultDelete &) = delete; // expected-error {{class member cannot be redeclared}}
};
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11875,8 +11875,12 @@
SkipBodyInfo *SkipBody) {
const FunctionDecl *Definition = EffectiveDefinition;
if (!Definition)
-if (!FD->isDefined(Definition))
+if (!FD->isOdrDefined(Definition))
return;
+ assert(Definition);
+
+ if (FD == Definition)
+return;
if (canRedefineFunction(Definition, getLangOpts()))
return;
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2534,8 +2534,18 @@
bool FunctionDecl::isDefined(const FunctionDecl *) const {
for (auto I : redecls()) {
-if (I->IsDeleted || I->IsDefaulted || I->Body || I->IsLateTemplateParsed ||
-I->hasDefiningAttr()) {
+if (I->isThisDeclarationADefinition()) {
+ Definition = I->IsDeleted ? I->getCanonicalDecl() : I;
+ return true;
+}
+ }
+
+ return false;
+}
+
+bool FunctionDecl::isOdrDefined(const FunctionDecl *) const {
+ for (auto I : redecls()) {
+if (I->isThisDeclarationAnOdrDefinition()) {
Definition = I->IsDeleted ? I->getCanonicalDecl() : I;
return true;
}
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -1778,11 +1778,26 @@
SourceRange getSourceRange() const override LLVM_READONLY;
- /// \brief Returns true if the function has a body (definition). The
- /// function body might be in any of the (re-)declarations of this
- /// function. The variant that accepts a FunctionDecl pointer will
- /// set that function declaration to the actual declaration
- /// containing the body (if there is one).
+ // Function definitions.
+ //
+ // A function declaration may be:
+ // - a non defining declaration,
+ // - a definition. A function may be defined because:
+ // - it has a body, or will have it in the case of late parsing.
+ // - it has an uninstantiated body. The body does not exist because the
+ // function is not used yet, but the declaration is considered a
+ // definition, it does not allow other definition of this function.
+ // - it does not have a user specified body, but it does not allow
+ // redefinition, because it is
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. In https://reviews.llvm.org/D3#760419, @jyu2 wrote: > In https://reviews.llvm.org/D3#760149, @aaron.ballman wrote: > > > As an FYI, there is a related check currently under development in > > clang-tidy; we probably should not duplicate this functionality in both > > places. See https://reviews.llvm.org/D19201 for the other review. > > > To my understanding, clang-tidy is kind of source check tool. It does more > intensive checking than compiler. > My purpose here is to emit minimum warning form compiler, in case, > clang-tidy is not used. Sort of correct. clang-tidy doesn't necessarily do more intensive checking than the compiler. It's more an AST matching source checking tool for checks that are either expensive or have a higher false-positive rate than we'd want to see in the frontend. I think that this particular check should probably be in the frontend, like you're doing. My biggest concern is that we do not duplicate functionality between the two tools. https://reviews.llvm.org/D19201 has has a considerable amount of review, so you should look at the test cases there and ensure you are handling them (including the fixits). Hopefully your patch ends up covering all of the functionality in the clang-tidy patch. > In https://reviews.llvm.org/D3#760353, @Prazek wrote: > >> Could you add similar tests as the ones that Stanislaw provied in his patch? >> Like the one with checking if throw is catched, or the conditional noexcept >> (by a macro, etc) > > > Good idea! Could add “marco” test for this. But I am not sure compiler want > to add check for throw caught by different catch handler. Because at compile > time, compiler can not statically determine which catch handler will be used > to caught the exception on all time. I would think that is pragma's > responsibility. > > For example: > > If (a) throw A else throw B; > > > > My main concern there is implicit noexcept gets set by compiler, which cause > runtime to termination. https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
jyu2 added a comment. In https://reviews.llvm.org/D3#760149, @aaron.ballman wrote: > As an FYI, there is a related check currently under development in > clang-tidy; we probably should not duplicate this functionality in both > places. See https://reviews.llvm.org/D19201 for the other review. To my understanding, clang-tidy is kind of source check tool. It does more intensive checking than compiler. My purpose here is to emit minimum warning form compiler, in case, clang-tidy is not used. In https://reviews.llvm.org/D3#760353, @Prazek wrote: > Could you add similar tests as the ones that Stanislaw provied in his patch? > Like the one with checking if throw is catched, or the conditional noexcept > (by a macro, etc) Good idea! Could add “marco” test for this. But I am not sure compiler want to add check for throw caught by different catch handler. Because at compile time, compiler can not statically determine which catch handler will be used to caught the exception on all time. I would think that is pragma's responsibility. For example: If (a) throw A else throw B; My main concern there is implicit noexcept gets set by compiler, which cause runtime to termination. https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33135: [ASTMatchers] Add support for floatLiterals
aaron.ballman added a comment.
In https://reviews.llvm.org/D33135#754278, @Lekensteyn wrote:
> By the way, I think that `long double` is less common than long unsigned
> literals, so changing unsigned to uint64_t might be something more important?
I agree that it's likely a more common use case. There doesn't appear to be a
suffix for __int128 (we talked about adding i128 once upon a time, but I don't
believe it got in), so you may be okay using a `uint64_t` there rather than an
`APInt`.
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:25
///:= true | false
+/// := 1.0 | 2e-3 | 3.45e67
/// := [0-9]+
Lekensteyn wrote:
> aaron.ballman wrote:
> > It'd be good to list the actual grammar rather than a few examples.
> I am concerned that listing a very precise grammar actually makes this less
> readable (see also the StringLiteral example).
>
> If a grammar is used instead, how about this:
>
> := [0-9]+.[0-9]* | [0-9]+.[0-9]*[eE][-+]?[0-9]+
>
That's reasonable enough.
Comment at: include/clang/ASTMatchers/Dynamic/VariantValue.h:335
unsigned Unsigned;
+double Double;
bool Boolean;
Lekensteyn wrote:
> aaron.ballman wrote:
> > This may or may not be a good idea, but do we want to put the values into
> > an APFloat rather than a double? My concern with double is that (0) it may
> > be subtly different if the user wants a 16- or 32-bit float explicitly, (1)
> > it won't be able to represent long double values, or quad double.
> >
> > I'm thinking this value could be passed directly from the C++ API as an
> > APFloat, float, or double, or provided using a StringRef for the dynamic
> > API.
> (32-bit) double values are a superset of (16-bit) float values, that should
> be OK.
> Long doubles are possible in the AST (e.g. for `0.1L`), but neither C11 nor
> C++14 seem to define a quad double literal type (so that should be of a
> lesser concern).
>
> Reasons why I chose for double instead of APFloat:
> - `strtod` is readily available and does not abort the program. By contrast,
> `APFloat(StringRef)` trips on assertions if the input is invalid.
> - I was not sure if the APFloat class can be used in an union.
The downside to using `strtod()` is that invalid input is silently accepted.
However, assertions on invalid input is certainly not good either. It might be
worth modifying `APFloat::convertFromString()` to accept invalid input and
return an error.
I think instead of an `APFloat`, maybe using an `APValue` for both the
`Unsigned` and `Double` fields might work. At the very least, it should give
you implementation ideas.
There is a quad double literal suffix: `q`. It's only supported on some
architectures, however. There are also imaginary numbers (`i`) and half (`h`).
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:180
/// \brief Consume an unsigned literal.
void consumeUnsignedLiteral(TokenInfo *Result) {
+bool isFloatingLiteral = false;
Lekensteyn wrote:
> aaron.ballman wrote:
> > This function should be renamed and the comment updated.
> How does "consumeNumberLiteral" sound?
Sounds good to me.
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:209
+ double doubleValue = strtod(Result->Text.str().c_str(), );
+ if (*end == 0) {
+Result->Kind = TokenInfo::TK_Literal;
You're failing to check errno here to ensure the value is actually valid.
https://reviews.llvm.org/D33135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33385: __cxa_demangle: Fix constructor cv qualifier handling
tberghammer created this revision.
Previously if we parsed a constructor then we set parsed_ctor_dtor_cv
to true and never reseted it. This causes issue when a template argument
references a constructor (e.g. type of lambda defined inside a
constructor) as we will have the parsed_ctor_dtor_cv flag set what will
cause issues when parsing later arguments.
https://reviews.llvm.org/D33385
Files:
src/cxa_demangle.cpp
test/test_demangle.pass.cpp
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29500,6 +29500,7 @@
{"_ZZ2f6vE1b", "f6()::b"},
{"_ZNV3$_35test9Ev", "$_3::test9() volatile"},
{"_ZN5Test8I3$_2EC1ES0_", "Test8<$_2>::Test8($_2)"},
+{"_Z3fooIZN3BarC1EvE3$_0EvT_", "void
foo(Bar::Bar()::$_0)"},
{"_ZGVZN1N1gEvE1a", "guard variable for N::g()::a"},
{"_ZplRK1YRA100_P1X", "operator+(Y const&, X* (&) [100])"},
{"_Z1fno", "f(__int128, unsigned __int128)"},
Index: src/cxa_demangle.cpp
===
--- src/cxa_demangle.cpp
+++ src/cxa_demangle.cpp
@@ -4564,6 +4564,8 @@
save_value sb(db.tag_templates);
if (db.encoding_depth > 1)
db.tag_templates = true;
+save_value
sp(db.parsed_ctor_dtor_cv);
+db.parsed_ctor_dtor_cv = false;
switch (*first)
{
case 'G':
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29500,6 +29500,7 @@
{"_ZZ2f6vE1b", "f6()::b"},
{"_ZNV3$_35test9Ev", "$_3::test9() volatile"},
{"_ZN5Test8I3$_2EC1ES0_", "Test8<$_2>::Test8($_2)"},
+{"_Z3fooIZN3BarC1EvE3$_0EvT_", "void foo(Bar::Bar()::$_0)"},
{"_ZGVZN1N1gEvE1a", "guard variable for N::g()::a"},
{"_ZplRK1YRA100_P1X", "operator+(Y const&, X* (&) [100])"},
{"_Z1fno", "f(__int128, unsigned __int128)"},
Index: src/cxa_demangle.cpp
===
--- src/cxa_demangle.cpp
+++ src/cxa_demangle.cpp
@@ -4564,6 +4564,8 @@
save_value sb(db.tag_templates);
if (db.encoding_depth > 1)
db.tag_templates = true;
+save_value sp(db.parsed_ctor_dtor_cv);
+db.parsed_ctor_dtor_cv = false;
switch (*first)
{
case 'G':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept
aaron.ballman added a comment. In https://reviews.llvm.org/D19201#760358, @Prazek wrote: > In https://reviews.llvm.org/D19201#760151, @aaron.ballman wrote: > > > As an FYI, there is a related check being implemented in clang currently; > > we probably should not duplicate this effort. See > > https://reviews.llvm.org/D3. > > > I think that clang is the right place for this feature, but I am not sure if > the patch has all the features (like checking if something will be catched, > or not showing warning for conditional noexcepts, which as we have seen > could be a problem for some projects. There also might be some other corner > cases that we didn't think about. > Assuming that this patch is ready to land, I would propose to send it to > trunk and remove it when the clang's patch will make it to the trunk. I am > not sure how much time it will take for other patch to be ready, but maybe we > could gather some usefull bug reports in the meantime and also Stanislaw > would have a contribution. I don't think it's a good idea to commit this and remove it when the frontend patch lands -- that's just extra code churn and runs the risk of breaking people who are using features out of trunk. I think the better approach is to find the correct home for the functionality based on what *both* patches are trying to accomplish, and combine the test cases from both patches to ensure we get the desired functionality. The two patches appear to be almost entirely overlapping, from my cursory look (of course, I may have missed something). I think that the frontend is likely the better place for this functionality to go in terms of where users might expect to find it, assuming the false-positive rate is reasonable and it isn't too computationally expensive. My recommendation would be to find the test cases in this patch that are not currently covered by the frontend patch, and ask that they be covered there (possibly even including the fixits). https://reviews.llvm.org/D19201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33170: [X86] Adding avx512_vpopcntdq feature set and its intrinsics
oren_ben_simhon added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:7526 +llvm::Type *ResultType = ConvertType(E->getType()); +llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, ResultType); +return Builder.CreateCall(F, Ops); oren_ben_simhon wrote: > craig.topper wrote: > > Why did the call to ConvertType come back? This code can just be > > > > > > ``` > > llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, Ops[0]->getType()); > > return Builder.CreateCall(F, Ops); > > ``` > After checking it again, I reverted that change because AFAIK Ops[0] is the > first argument and not the return type of the statement. After checking it again, I reverted that change because AFAIK Ops[0] is the first argument and not the return type of the statement. Repository: rL LLVM https://reviews.llvm.org/D33170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33170: [X86] Adding avx512_vpopcntdq feature set and its intrinsics
oren_ben_simhon added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:7527 +Value *X = EmitScalarExpr(E->getArg(0)); +llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, ResultType); +return Builder.CreateCall(F, X); craig.topper wrote: > I think you can use Ops[0]->getType() and avoid the call to ConvertType. See > the code for BI__builtin_ia32_vplzcntq_512_mask. After checking it again, I am going to revert this change because AFAIK Ops[0] is the first argument and the return type. Comment at: lib/CodeGen/CGBuiltin.cpp:7526 +llvm::Type *ResultType = ConvertType(E->getType()); +llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, ResultType); +return Builder.CreateCall(F, Ops); craig.topper wrote: > Why did the call to ConvertType come back? This code can just be > > > ``` > llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, Ops[0]->getType()); > return Builder.CreateCall(F, Ops); > ``` After checking it again, I reverted that change because AFAIK Ops[0] is the first argument and not the return type of the statement. Repository: rL LLVM https://reviews.llvm.org/D33170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept
Prazek accepted this revision. Prazek added a comment. LGTM, but wait if Aaron will accept it. https://reviews.llvm.org/D19201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept
Prazek added a comment. In https://reviews.llvm.org/D19201#760151, @aaron.ballman wrote: > As an FYI, there is a related check being implemented in clang currently; we > probably should not duplicate this effort. See > https://reviews.llvm.org/D3. I think that clang is the right place for this feature, but I am not sure if the patch has all the features (like checking if something will be catched, or not showing warning for conditional noexcepts, which as we have seen could be a problem for some projects. There also might be some other corner cases that we didn't think about. Assuming that this patch is ready to land, I would propose to send it to trunk and remove it when the clang's patch will make it to the trunk. I am not sure how much time it will take for other patch to be ready, but maybe we could gather some usefull bug reports in the meantime and also Stanislaw would have a contribution. https://reviews.llvm.org/D19201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
Prazek added a comment. Could you add similar tests as the ones that Stanislaw provied in his patch? Like the one with checking if throw is catched, or the conditional noexcept (by a macro, etc) https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33170: [X86] Adding avx512_vpopcntdq feature set and its intrinsics
craig.topper added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:7526 +llvm::Type *ResultType = ConvertType(E->getType()); +llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, ResultType); +return Builder.CreateCall(F, Ops); Why did the call to ConvertType come back? This code can just be ``` llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, Ops[0]->getType()); return Builder.CreateCall(F, Ops); ``` Repository: rL LLVM https://reviews.llvm.org/D33170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303501 - [Format] Add curly braces to suppress a -Wmisleading-indentation warning from gcc.
Author: ctopper
Date: Sun May 21 02:29:07 2017
New Revision: 303501
URL: http://llvm.org/viewvc/llvm-project?rev=303501=rev
Log:
[Format] Add curly braces to suppress a -Wmisleading-indentation warning from
gcc.
Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=303501=303500=303501=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Sun May 21 02:29:07 2017
@@ -576,12 +576,13 @@ private:
}
break;
case tok::kw_for:
- if (Style.Language == FormatStyle::LK_JavaScript)
+ if (Style.Language == FormatStyle::LK_JavaScript) {
if (Tok->Previous && Tok->Previous->is(tok::period))
break;
// JS' for await ( ...
if (CurrentToken && CurrentToken->is(Keywords.kw_await))
next();
+ }
Contexts.back().ColonIsForRangeExpr = true;
next();
if (!parseParens())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33170: [X86] Adding avx512_vpopcntdq feature set and its intrinsics
oren_ben_simhon updated this revision to Diff 99685.
oren_ben_simhon marked an inline comment as done.
oren_ben_simhon added a comment.
Implemented comments posted until 05/20 (Thanks Craig)
Repository:
rL LLVM
https://reviews.llvm.org/D33170
Files:
include/clang/Basic/BuiltinsX86.def
include/clang/Driver/Options.td
lib/Basic/Targets.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Headers/CMakeLists.txt
lib/Headers/avx512vpopcntdqintrin.h
lib/Headers/immintrin.h
test/CodeGen/attr-target-x86.c
test/CodeGen/avx512vpopcntdqintrin.c
Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -7,6 +7,7 @@
avx2intrin.h
avx512bwintrin.h
avx512cdintrin.h
+ avx512vpopcntdqintrin.h
avx512dqintrin.h
avx512erintrin.h
avx512fintrin.h
Index: lib/Headers/avx512vpopcntdqintrin.h
===
--- lib/Headers/avx512vpopcntdqintrin.h
+++ lib/Headers/avx512vpopcntdqintrin.h
@@ -0,0 +1,70 @@
+/*===- avx512vpopcntdqintrin.h - AVX512VPOPCNTDQ intrinsics
+ *--===
+ *
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ *===---===
+ */
+#ifndef __IMMINTRIN_H
+#error \
+"Never use directly; include instead."
+#endif
+
+#ifndef __AVX512VPOPCNTDQINTRIN_H
+#define __AVX512VPOPCNTDQINTRIN_H
+
+/* Define the default attributes for the functions in this file. */
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("avx512vpopcntd" \
+"q")))
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS _mm512_popcnt_epi64(__m512i __A) {
+ return (__m512i)__builtin_ia32_vpopcntq_512((__v8di)__A);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_mask_popcnt_epi64(__m512i __W, __mmask8 __U, __m512i __A) {
+ return (__m512i)__builtin_ia32_selectq_512(
+ (__mmask8)__U, (__v8di)_mm512_popcnt_epi64(__A), (__v8di)__W);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_maskz_popcnt_epi64(__mmask8 __U, __m512i __A) {
+ return _mm512_mask_popcnt_epi64((__m512i)_mm512_setzero_si512(), __U, __A);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS _mm512_popcnt_epi32(__m512i __A) {
+ return (__m512i)__builtin_ia32_vpopcntd_512((__v16si)__A);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_mask_popcnt_epi32(__m512i __W, __mmask16 __U, __m512i __A) {
+ return (__m512i)__builtin_ia32_selectd_512(
+ (__mmask16)__U, (__v16si)_mm512_popcnt_epi32(__A), (__v16si)__W);
+}
+
+static __inline__ __m512i __DEFAULT_FN_ATTRS
+_mm512_maskz_popcnt_epi32(__mmask16 __U, __m512i __A) {
+ return _mm512_mask_popcnt_epi32((__m512i)_mm512_setzero_si512(), __U, __A);
+}
+
+#undef __DEFAULT_FN_ATTRS
+
+#endif
Index: lib/Headers/immintrin.h
===
--- lib/Headers/immintrin.h
+++ lib/Headers/immintrin.h
@@ -146,6 +146,10 @@
#include
#endif
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__AVX512VPOPCNTDQ__)
+#include
+#endif
+
#if !defined(_MSC_VER) || __has_feature(modules) || defined(__AVX512DQ__)
#include
#endif
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -7332,39 +7332,42 @@
AVX512PF,
AVX512VBMI,
AVX512IFMA,
+ AVX512VPOPCNTDQ,
MAX
};
-X86Features Feature = StringSwitch(FeatureStr)
- .Case("cmov", X86Features::CMOV)
- .Case("mmx", X86Features::MMX)
- .Case("popcnt",
