[PATCH] D37941: [X86] Move even more of our CPU to feature mapping switch to use fallthroughs
RKSimon accepted this revision. RKSimon added a comment. This revision is now accepted and ready to land. LGTM, there are a couple of other fall-throughs that could still be done. Comment at: lib/Basic/Targets/X86.cpp:141 setFeatureEnabledImpl(Features, "sse2", true); setFeatureEnabledImpl(Features, "fxsr", true); break; CK_Pentium3/CK_C3_2/CK_PentiumM/CK_Pentium4/CK_x86_64 can be put below Yonah et al. (unless you're worried about including non-Intel enums)? https://reviews.llvm.org/D37941 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37938: [X86] Remove unnecessary extra encodings from the CPU name enum in clang
RKSimon accepted this revision. RKSimon added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D37938 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check
JonasToth added a comment.
I built the patch locally, here is my output:
The Unittest failed for me: `Neither CHECK-FIXES nor CHECK-MESSAGES found in
the input`
Script call:
`/usr/bin/python2.7
/home/jonas/opt/llvm/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py
/home/jonas/opt/llvm/tools/clang/tools/extra/test/clang-tidy/readability-function-cognitive-complexity.cpp
readability-function-cognitive-complexity
/home/jonas/opt/llvm/build/tools/clang/tools/extra/test/clang-tidy/Output/readability-function-cognitive-complexity.cpp.tmp
-- -config='{CheckOptions: [{key:
readability-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11`
Building the docs failed as well, i added inline comment on that.
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:62
+std::pair Process() const {
+ assert(C != Criteria::None && "invalid criteria");
+
lebedev.ri wrote:
> JonasToth wrote:
> > You acces `Critera` always scoped. I think you could declare `Criteria` to
> > be a `enum class`, not just a plain `enum`
> Apparently it's not as simple as that...
> Then i get:
> ```
> 1/8] Building CXX object
> tools/clang/tools/extra/clang-tidy/readability/CMakeFiles/clangTidyReadabilityModule.dir/FunctionCognitiveComplexityCheck.cpp.o
> FAILED:
> tools/clang/tools/extra/clang-tidy/readability/CMakeFiles/clangTidyReadabilityModule.dir/FunctionCognitiveComplexityCheck.cpp.o
>
> /usr/local/bin/clang++ -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
> -Itools/clang/tools/extra/clang-tidy/readability
> -I/build/clang-tools-extra/clang-tidy/readability -I/build/clang/include
> -Itools/clang/include -I/usr/include/libxml2 -Iinclude -I/build/llvm/include
> -fPIC -fvisibility-inlines-hidden -Werror=date-time
> -Werror=unguarded-availability-new -std=c++11 -Wall -W -Wno-unused-parameter
> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic
> -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor
> -Wdelete-non-virtual-dtor -Wstring-conversion -fcolor-diagnostics
> -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual
> -Wno-nested-anon-types -O3 -DNDEBUG-fno-exceptions -fno-rtti -MD -MT
> tools/clang/tools/extra/clang-tidy/readability/CMakeFiles/clangTidyReadabilityModule.dir/FunctionCognitiveComplexityCheck.cpp.o
> -MF
> tools/clang/tools/extra/clang-tidy/readability/CMakeFiles/clangTidyReadabilityModule.dir/FunctionCognitiveComplexityCheck.cpp.o.d
> -o
> tools/clang/tools/extra/clang-tidy/readability/CMakeFiles/clangTidyReadabilityModule.dir/FunctionCognitiveComplexityCheck.cpp.o
> -c
> /build/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
> /build/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:100:44:
> error: invalid operands to binary expression
> ('clang::tidy::readability::(anonymous
> namespace)::CognitiveComplexity::Criteria' and
> 'clang::tidy::readability::(anonymous
> namespace)::CognitiveComplexity::Criteria')
> } else if (C == (Criteria::Increment | Criteria::IncrementNesting)) {
>~~~ ^ ~~
> /build/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:536:9:
> error: value of type 'CognitiveComplexity::Criteria' is not contextually
> convertible to 'bool'
> if (Reasons & CognitiveComplexity::Criteria::All)
> ^~~~
> 2 errors generated.
> ninja: build stopped: subcommand failed.
>
> ```
>
> The first failure i could have explained as use-before-declaration. But not
> the second one
> I'll keep it as is where it is for now.
AFAIK `enum class` does not behave like a 'tagged integer'. It is necessary to
to implement all operators (which you did), but i think implicit conversion are
not allowed as well.
This would explain both the first and second error message.
Leave it as is, since the usage is limited to only this file.
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102-114
+const std::array CognitiveComplexity::Msgs = {{
+// B1 + B2 + B3
+"+%0, including nesting penalty of %1, nesting level increased to %2",
+
+// B1 + B2
+"+%0, nesting level increased to %2",
+
lebedev.ri wrote:
> JonasToth wrote:
> > could this initialization land in line 45? that would be directly close to
> > the criteria.
> It would be nice indeed, but if i do
> ```
> // All the possible messages that can be outputed. The choice of the message
> // to use is based of the combination of the Criterias
> static constexpr std::array Msgs = {{
> // B1 + B2 + B3
> "+%0, including nesting penalty of %1, nesting level increased to %2",
>
> // B1 + B2
> "+%0, nesting level increased to %2",
>
> // B1
> "+
[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check
lebedev.ri added a comment.
In https://reviews.llvm.org/D36836#873032, @JonasToth wrote:
> I built the patch locally, here is my output:
>
> The Unittest failed for me: `Neither CHECK-FIXES nor CHECK-MESSAGES found in
> the input`
>
> Script call:
> `/usr/bin/python2.7
> /home/jonas/opt/llvm/tools/clang/tools/extra/test/../test/clang-tidy/check_clang_tidy.py
>
> /home/jonas/opt/llvm/tools/clang/tools/extra/test/clang-tidy/readability-function-cognitive-complexity.cpp
> readability-function-cognitive-complexity
> /home/jonas/opt/llvm/build/tools/clang/tools/extra/test/clang-tidy/Output/readability-function-cognitive-complexity.cpp.tmp
> -- -config='{CheckOptions: [{key:
> readability-function-cognitive-complexity.Threshold, value: 0}]}' --
> -std=c++11`
You did note that https://reviews.llvm.org/D36892 needs to be applied first?
> Building the docs failed as well, i added inline comment on that.
Repository:
rL LLVM
https://reviews.llvm.org/D36836
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check
JonasToth added a comment. I see, forgot that. The doc were the interesting part for me, I trust that you test the code correctlty :) Repository: rL LLVM https://reviews.llvm.org/D36836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches
JonasToth added a comment. In https://reviews.llvm.org/D37808#869810, @Eugene.Zelenko wrote: > In https://reviews.llvm.org/D37808#869612, @JonasToth wrote: > > > In https://reviews.llvm.org/D37808#869602, @Eugene.Zelenko wrote: > > > > > I think will be good idea to extend -Wswitch diagnostics. > > > > > > Ok. But it will introduce new warnings to llvm codebase itself. I prepare > > some example output i found right now. > > > If number of them will not be huge, it'll be worth to fix before extended > -Wswitch will be committed. Where would be the correct place to further discuss extending -Wswitch ? Should i ask that on the mailing list? https://reviews.llvm.org/D37808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33514: [WIP] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
chapuni added a comment. Fixed in https://reviews.llvm.org/rL313456. Comment at: include/llvm/IR/DiagnosticHandler.h:12 +//===--===// + +#include "llvm/ADT/StringRef.h" Did you forget include guard here? https://reviews.llvm.org/D33514 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches
lebedev.ri added a comment.
In https://reviews.llvm.org/D37808#869810, @Eugene.Zelenko wrote:
> In https://reviews.llvm.org/D37808#869612, @JonasToth wrote:
>
> > In https://reviews.llvm.org/D37808#869602, @Eugene.Zelenko wrote:
> >
> > > I think will be good idea to extend -Wswitch diagnostics.
> >
> >
> > Ok. But it will introduce new warnings to llvm codebase itself. I prepare
> > some example output i found right now.
>
>
> If number of them will not be huge, it'll be worth to fix before extended
> -Wswitch will be committed.
The other way around, all new warnings must to be fixed first, before
committing the new diagnostic itself.
I might agree that the part of this check that is handling `switch()` might
indeed be better off extending `-Wswitch` (as several new flags
`-Wswitch-missing-default`, `-Wswitch-empty`, `-Wswitch-prefer-if`, or
something like that)
But then a much greater thought shall be put into ensuring quality of the new
diagnostic.
Oh, and i'd say the `WarnOnMissingElse` should *not* be moved to clang
diagnostic. At least right now it will result in many false-positives.
E.g. what would it say about
void maybefalse(int i) {
if (i > 0) {
return;
} else if (i <= 0) {
return;
}
https://reviews.llvm.org/D37808
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches
JonasToth added a comment. Informationen my part think that both switch and else should stay here for now. Running it over llvm gave really a lot of warnings, and that a rather good codebase. The check did not recognize logic, your example would be warned against. I don't see a way to implement something like that here, that would require condition solving and so on. In general, this check is more about defensive programming and therefore clang-tidy a good place. https://reviews.llvm.org/D37808 ___ 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 115534.
sepavloff added a comment.
Rebased patch.
https://reviews.llvm.org/D30170
Files:
include/clang/AST/Decl.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantiateDecl.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: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1807,45 +1807,24 @@
// apply to non-template function declarations and definitions also apply
// to these implicit definitions.
if (D->isThisDeclarationADefinition()) {
- // Check for a function body.
- const FunctionDecl *Definition = nullptr;
- if (Function->isDefined(Definition) &&
- Definition->getTemplateSpecializationKind() == TSK_Undeclared) {
-SemaRef.Diag(Function->getLocation(), diag::err_redefinition)
-<< Function->getDeclName();
-SemaRef.Diag(Definition->getLocation(), diag::note_previous_definition);
- }
- // Check for redefinitions due to other instantiations of this or
- // a similar friend function.
- else for (auto R : Function->redecls()) {
-if (R == Function)
- continue;
-
-// If some prior declaration of this function has been used, we need
-// to instantiate its definition.
-if (!QueuedInstantiation && R->isUsed(false)) {
- if (MemberSpecializationInfo *MSInfo =
- Function->getMemberSpecializationInfo()) {
-if (MSInfo->getPointOfInstantiation().isInvalid()) {
- SourceLocation Loc = R->getLocation(); // FIXME
- MSInfo->setPointOfInstantiation(Loc);
- SemaRef.PendingLocalImplicitInstantiations.push_back(
- std::make_pair(Function, Loc));
- QueuedInstantiation = true;
-}
- }
-}
-
-// If some prior declaration of this function was a friend with an
-// uninstantiated definition, reject it.
-if (R->getFriendObjectKind()) {
- if (const FunctionDecl *RPattern =
- R->getTemplateInstantiationPattern()) {
-if (RPattern->isDefined(RPattern)) {
- SemaRef.Diag(Function->getLocation(), diag::err_redefinition)
-<< Function->getDeclName();
- SemaRef.Diag(R->getLocation(), diag::note_previous_definition);
- break;
+ SemaRef.CheckForFunctionRedefinition(Function);
+ if (!Function->isInvalidDecl()) {
+for (auto R : Function->redecls()) {
+ if (R == Function)
+continue;
+
+ // If some prior declaration of this function has been used, we need
+ // to instantiate its definition.
+ if (!QueuedInstantiation && R->isUsed(false)) {
+if (MemberSpecializationInfo *MSInfo =
+Function->getMemberSpecializationInfo()) {
+ if (MSInfo->getPointOfInstantiation().isInvalid()) {
+SourceLocation Loc = R->getLocation(); // FIXME
+MSInfo->setPointOfInstantiation(Loc);
+SemaRef.PendingLocalImplicitInstantiations.push_back(
+std::make_pair(Function, Loc));
+QueuedInstantiation = true;
+ }
}
}
}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11981,9 +11981,31 @@
const FunctionDecl *EffectiveDefinition,
SkipBodyInfo *SkipBody) {
const FunctionDecl *Definition = EffectiveDefinition;
+ if (!Definition &&
+ !FD->is
Re: r313315 - Diagnostic specific failed condition in a static_assert.
This triggered failure in libcxx tests.
http://bb.pgr.jp/builders/bootstrap-clang-libcxx-lld-i686-linux/builds/97
On Fri, Sep 15, 2017 at 8:40 AM Douglas Gregor via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: dgregor
> Date: Thu Sep 14 16:38:42 2017
> New Revision: 313315
>
> URL: http://llvm.org/viewvc/llvm-project?rev=313315&view=rev
> Log:
> Diagnostic specific failed condition in a static_assert.
>
> When a static_assert fails, dig out a specific condition to diagnose,
> using the same logic that we use to find the enable_if condition to
> diagnose.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaTemplate.cpp
> cfe/trunk/test/SemaCXX/static-assert.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313315&r1=313314&r2=313315&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 14
> 16:38:42 2017
> @@ -1219,6 +1219,8 @@ def warn_messaging_unqualified_id : Warn
> def err_static_assert_expression_is_not_constant : Error<
>"static_assert expression is not an integral constant expression">;
> def err_static_assert_failed : Error<"static_assert failed%select{
> %1|}0">;
> +def err_static_assert_requirement_failed : Error<
> + "static_assert failed due to requirement '%0'%select{ %2|}1">;
> def ext_static_assert_no_message : ExtWarn<
>"static_assert with no message is a C++17 extension">, InGroup;
> def warn_cxx14_compat_static_assert_no_message : Warning<
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=313315&r1=313314&r2=313315&view=diff
>
> ==
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 14 16:38:42 2017
> @@ -2783,6 +2783,14 @@ public:
>EnableIfAttr *CheckEnableIf(FunctionDecl *Function, ArrayRef
> Args,
>bool MissingImplicitThis = false);
>
> + /// Find the failed Boolean condition within a given Boolean
> + /// constant expression, and describe it with a string.
> + ///
> + /// \param AllowTopLevelCond Whether to allow the result to be the
> + /// complete top-level condition.
> + std::pair
> + findFailedBooleanCondition(Expr *Cond, bool AllowTopLevelCond);
> +
>/// Emit diagnostics for the diagnose_if attributes on Function,
> ignoring any
>/// non-ArgDependent DiagnoseIfAttrs.
>///
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=313315&r1=313314&r2=313315&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 14 16:38:42 2017
> @@ -13296,8 +13296,20 @@ Decl *Sema::BuildStaticAssertDeclaration
>llvm::raw_svector_ostream Msg(MsgBuffer);
>if (AssertMessage)
> AssertMessage->printPretty(Msg, nullptr, getPrintingPolicy());
> - Diag(StaticAssertLoc, diag::err_static_assert_failed)
> -<< !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
> +
> + Expr *InnerCond = nullptr;
> + std::string InnerCondDescription;
> + std::tie(InnerCond, InnerCondDescription) =
> +findFailedBooleanCondition(Converted.get(),
> + /*AllowTopLevelCond=*/false);
> + if (InnerCond) {
> +Diag(StaticAssertLoc, diag::err_static_assert_requirement_failed)
> + << InnerCondDescription << !AssertMessage
> + << Msg.str() << InnerCond->getSourceRange();
> + } else {
> +Diag(StaticAssertLoc, diag::err_static_assert_failed)
> + << !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
> + }
>Failed = true;
> }
>}
>
> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=313315&r1=313314&r2=313315&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Thu Sep 14 16:38:42 2017
> @@ -2863,11 +2863,9 @@ static Expr *lookThroughRangesV3Conditio
>return Cond;
> }
>
> -/// Find the failed subexpression within enable_if, and describe it
> -/// with a string.
> -static std::pair
> -findFailedEnableIfCondition(Sema &S, Expr *Cond) {
> - Cond = lookThroughRangesV3Condition(S.PP, Cond);
> +std::pair
> +Sema::findF
[Diffusion] rL302247: Introduce Wzero-as-null-pointer-constant.
lebedev.ri added subscribers: thakis, cfe-commits, dblaikie, malcolm.parsons, hans, lebedev.ri. lebedev.ri raised a concern with this commit. lebedev.ri added a comment. Hi @thakis! Are you planning on addressing the kind-of false-positives this diagnostic produces? It should not warn if `0` is: - From system header - From macro, especially if it is defined in a system header - Is used as an initializer for dependent variable (`template void fun(T var = 0)` should not warn) Refs. https://bugs.llvm.org/show_bug.cgi?id=33771 Refs. https://reviews.llvm.org/D32914 Users: lebedev.ri (Auditor) https://reviews.llvm.org/rL302247 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21508: Diagnose friend function template redefinitions
sepavloff updated this revision to Diff 115539.
sepavloff added a comment.
Rebased
https://reviews.llvm.org/D21508
Files:
include/clang/AST/DeclBase.h
lib/AST/Decl.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/SemaCXX/friend2.cpp
Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -129,6 +129,83 @@
void func_22() {} // expected-error{{redefinition of 'func_22'}}
+// Case of template friend functions.
+
+template void func_31(T *x);
+template
+struct C31a {
+ template friend void func_31(T *x) {}
+};
+template
+struct C31b {
+ template friend void func_31(T *x) {}
+};
+
+
+template inline void func_32(T *x) {}
+template
+struct C32a {
+ template friend void func_32(T *x) {}
+};
+template
+struct C32b {
+ template friend void func_32(T *x) {}
+};
+
+
+template
+struct C33a {
+ template friend void func_33(T *x) {}
+};
+template
+struct C33b {
+ template friend void func_33(T *x) {}
+};
+
+
+template inline void func_34(T *x) {} // expected-note{{previous definition is here}}
+template
+struct C34 {
+ template friend void func_34(T *x) {} // expected-error{{redefinition of 'func_34'}}
+};
+
+C34 v34; // expected-note{{in instantiation of template class 'C34' requested here}}
+
+
+template inline void func_35(T *x);
+template
+struct C35a {
+ template friend void func_35(T *x) {} // expected-note{{previous definition is here}}
+};
+template
+struct C35b {
+ template friend void func_35(T *x) {} // expected-error{{redefinition of 'func_35'}}
+};
+
+C35a v35a;
+C35b v35b; // expected-note{{in instantiation of template class 'C35b' requested here}}
+
+
+template void func_36(T *x);
+template
+struct C36 {
+ template friend void func_36(T *x) {} // expected-error{{redefinition of 'func_36'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+C36 v36a;
+C36 v36b; //expected-note{{in instantiation of template class 'C36' requested here}}
+
+
+template void func_37(T *x);
+template
+struct C37 {
+ template friend void func_37(T *x) {} // expected-note{{previous definition is here}}
+};
+
+C37 v37;
+template void func_37(T *x) {} // expected-error{{redefinition of 'func_37'}}
+
namespace pr22307 {
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1795,7 +1795,9 @@
// If the original function was part of a friend declaration,
// inherit its namespace state and add it to the owner.
if (isFriend) {
-PrincipalDecl->setObjectOfFriendDecl();
+Function->setObjectOfFriendDecl();
+if (FunctionTemplate)
+ FunctionTemplate->setObjectOfFriendDecl();
DC->makeDeclVisibleInContext(PrincipalDecl);
bool QueuedInstantiation = false;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12004,6 +12004,29 @@
}
}
}
+
+ if (!Definition)
+// Similar to friend functions a friend function template may be a
+// definition and do not have a body if it is instantiated in a class
+// template.
+if (FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate()) {
+ for (auto I : FTD->redecls()) {
+auto D = cast(I);
+if (D != FTD) {
+ assert(!D->isThisDeclarationADefinition() &&
+ "Underlying function declaration must be a definition");
+ if (D->getFriendObjectKind() != Decl::FOK_None)
+if (FunctionTemplateDecl *FT =
+ D->getInstantiatedFromMemberTemplate()) {
+ if (FT->isThisDeclarationADefinition()) {
+Definition = D->getTemplatedDecl();
+break;
+ }
+}
+}
+ }
+}
+
if (!Definition)
return;
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -3267,6 +3267,14 @@
if (auto *MFD = getInstantiatedFromMemberFunction())
return getDefinitionOrSelf(MFD);
+ if (FunctionTemplateDecl *TD = getDescribedFunctionTemplate()) {
+if (TD->getFriendObjectKind() != FOK_None) {
+ while (FunctionTemplateDecl *FT = TD->getInstantiatedFromMemberTemplate())
+TD = FT;
+ return TD->getTemplatedDecl();
+}
+ }
+
return nullptr;
}
Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1034,11 +1034,11 @@
unsigned OldNS = IdentifierNamespace;
assert((OldNS & (IDNS_Tag | IDNS_Ordinary |
IDNS_TagFriend | IDNS_OrdinaryFriend |
- IDNS_
r313462 - [X86] Remove unnecessary extra encodings from the CPU name enum in clang
Author: ctopper
Date: Sat Sep 16 09:44:39 2017
New Revision: 313462
URL: http://llvm.org/viewvc/llvm-project?rev=313462&view=rev
Log:
[X86] Remove unnecessary extra encodings from the CPU name enum in clang
Summary:
For a lot of older CPUs we have a 1:1 mapping between CPU name and enum name.
But many of them are effectively aliases of each other and as a result are
always repeated together at every usage
This patch removes most of the duplication. It also uses StringSwitch::Cases to
make the many to one mapping in the StringSwitch more obvious.
Reviewers: RKSimon, spatel, zvi, igorb
Reviewed By: RKSimon
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D37938
Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h
Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=313462&r1=313461&r2=313462&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Sat Sep 16 09:44:39 2017
@@ -130,14 +130,12 @@ bool X86TargetInfo::initFeatureMap(
setFeatureEnabledImpl(Features, "mmx", true);
break;
case CK_Pentium3:
- case CK_Pentium3M:
case CK_C3_2:
setFeatureEnabledImpl(Features, "sse", true);
setFeatureEnabledImpl(Features, "fxsr", true);
break;
case CK_PentiumM:
case CK_Pentium4:
- case CK_Pentium4M:
case CK_x86_64:
setFeatureEnabledImpl(Features, "sse2", true);
setFeatureEnabledImpl(Features, "fxsr", true);
@@ -265,21 +263,15 @@ bool X86TargetInfo::initFeatureMap(
setFeatureEnabledImpl(Features, "3dnow", true);
break;
case CK_Athlon:
- case CK_AthlonThunderbird:
case CK_Geode:
setFeatureEnabledImpl(Features, "3dnowa", true);
break;
- case CK_Athlon4:
case CK_AthlonXP:
- case CK_AthlonMP:
setFeatureEnabledImpl(Features, "sse", true);
setFeatureEnabledImpl(Features, "3dnowa", true);
setFeatureEnabledImpl(Features, "fxsr", true);
break;
case CK_K8:
- case CK_Opteron:
- case CK_Athlon64:
- case CK_AthlonFX:
setFeatureEnabledImpl(Features, "sse2", true);
setFeatureEnabledImpl(Features, "3dnowa", true);
setFeatureEnabledImpl(Features, "fxsr", true);
@@ -290,8 +282,6 @@ bool X86TargetInfo::initFeatureMap(
setFeatureEnabledImpl(Features, "popcnt", true);
LLVM_FALLTHROUGH;
case CK_K8SSE3:
- case CK_OpteronSSE3:
- case CK_Athlon64SSE3:
setFeatureEnabledImpl(Features, "sse3", true);
setFeatureEnabledImpl(Features, "3dnowa", true);
setFeatureEnabledImpl(Features, "fxsr", true);
@@ -807,7 +797,6 @@ void X86TargetInfo::getTargetDefines(con
defineCPUMacros(Builder, "pentium");
break;
case CK_Pentium3:
- case CK_Pentium3M:
case CK_PentiumM:
Builder.defineMacro("__tune_pentium3__");
LLVM_FALLTHROUGH;
@@ -827,7 +816,6 @@ void X86TargetInfo::getTargetDefines(con
Builder.defineMacro("__pentiumpro__");
break;
case CK_Pentium4:
- case CK_Pentium4M:
defineCPUMacros(Builder, "pentium4");
break;
case CK_Yonah:
@@ -888,10 +876,7 @@ void X86TargetInfo::getTargetDefines(con
defineCPUMacros(Builder, "k6");
break;
case CK_Athlon:
- case CK_AthlonThunderbird:
- case CK_Athlon4:
case CK_AthlonXP:
- case CK_AthlonMP:
defineCPUMacros(Builder, "athlon");
if (SSELevel != NoSSE) {
Builder.defineMacro("__athlon_sse__");
@@ -901,11 +886,6 @@ void X86TargetInfo::getTargetDefines(con
case CK_K8:
case CK_K8SSE3:
case CK_x86_64:
- case CK_Opteron:
- case CK_OpteronSSE3:
- case CK_Athlon64:
- case CK_Athlon64SSE3:
- case CK_AthlonFX:
defineCPUMacros(Builder, "k8");
break;
case CK_AMDFAM10:
@@ -1553,55 +1533,37 @@ X86TargetInfo::CPUKind X86TargetInfo::ge
.Case("i686", CK_i686)
.Case("pentiumpro", CK_PentiumPro)
.Case("pentium2", CK_Pentium2)
- .Case("pentium3", CK_Pentium3)
- .Case("pentium3m", CK_Pentium3M)
+ .Cases("pentium3", "pentium3m", CK_Pentium3)
.Case("pentium-m", CK_PentiumM)
.Case("c3-2", CK_C3_2)
.Case("yonah", CK_Yonah)
- .Case("pentium4", CK_Pentium4)
- .Case("pentium4m", CK_Pentium4M)
+ .Cases("pentium4", "pentium4m", CK_Pentium4)
.Case("prescott", CK_Prescott)
.Case("nocona", CK_Nocona)
.Case("core2", CK_Core2)
.Case("penryn", CK_Penryn)
- .Case("bonnell", CK_Bonnell)
- .Case("atom", CK_Bonnell) // Legacy name.
- .Case("silvermont", CK_Silvermont)
- .Case("slm", CK_Silvermont) // Legacy name.
+ .Cases("bonnell", "atom", CK_Bonnell)
+ .Cases("silvermont", "slm", CK_Silvermont)
.Case("goldmont", CK_Goldmont)
- .Case("nehalem", CK_Nehalem)
- .Case("corei7", CK_Nehalem) // Legacy name.
+ .Cases("nehalem", "corei7", CK_Nehalem)
.Case("westmere", CK_Westmere)
- .C
[PATCH] D37938: [X86] Remove unnecessary extra encodings from the CPU name enum in clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313462: [X86] Remove unnecessary extra encodings from the
CPU name enum in clang (authored by ctopper).
Changed prior to commit:
https://reviews.llvm.org/D37938?vs=115512&id=115540#toc
Repository:
rL LLVM
https://reviews.llvm.org/D37938
Files:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h
Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -130,14 +130,12 @@
setFeatureEnabledImpl(Features, "mmx", true);
break;
case CK_Pentium3:
- case CK_Pentium3M:
case CK_C3_2:
setFeatureEnabledImpl(Features, "sse", true);
setFeatureEnabledImpl(Features, "fxsr", true);
break;
case CK_PentiumM:
case CK_Pentium4:
- case CK_Pentium4M:
case CK_x86_64:
setFeatureEnabledImpl(Features, "sse2", true);
setFeatureEnabledImpl(Features, "fxsr", true);
@@ -265,21 +263,15 @@
setFeatureEnabledImpl(Features, "3dnow", true);
break;
case CK_Athlon:
- case CK_AthlonThunderbird:
case CK_Geode:
setFeatureEnabledImpl(Features, "3dnowa", true);
break;
- case CK_Athlon4:
case CK_AthlonXP:
- case CK_AthlonMP:
setFeatureEnabledImpl(Features, "sse", true);
setFeatureEnabledImpl(Features, "3dnowa", true);
setFeatureEnabledImpl(Features, "fxsr", true);
break;
case CK_K8:
- case CK_Opteron:
- case CK_Athlon64:
- case CK_AthlonFX:
setFeatureEnabledImpl(Features, "sse2", true);
setFeatureEnabledImpl(Features, "3dnowa", true);
setFeatureEnabledImpl(Features, "fxsr", true);
@@ -290,8 +282,6 @@
setFeatureEnabledImpl(Features, "popcnt", true);
LLVM_FALLTHROUGH;
case CK_K8SSE3:
- case CK_OpteronSSE3:
- case CK_Athlon64SSE3:
setFeatureEnabledImpl(Features, "sse3", true);
setFeatureEnabledImpl(Features, "3dnowa", true);
setFeatureEnabledImpl(Features, "fxsr", true);
@@ -807,7 +797,6 @@
defineCPUMacros(Builder, "pentium");
break;
case CK_Pentium3:
- case CK_Pentium3M:
case CK_PentiumM:
Builder.defineMacro("__tune_pentium3__");
LLVM_FALLTHROUGH;
@@ -827,7 +816,6 @@
Builder.defineMacro("__pentiumpro__");
break;
case CK_Pentium4:
- case CK_Pentium4M:
defineCPUMacros(Builder, "pentium4");
break;
case CK_Yonah:
@@ -888,10 +876,7 @@
defineCPUMacros(Builder, "k6");
break;
case CK_Athlon:
- case CK_AthlonThunderbird:
- case CK_Athlon4:
case CK_AthlonXP:
- case CK_AthlonMP:
defineCPUMacros(Builder, "athlon");
if (SSELevel != NoSSE) {
Builder.defineMacro("__athlon_sse__");
@@ -901,11 +886,6 @@
case CK_K8:
case CK_K8SSE3:
case CK_x86_64:
- case CK_Opteron:
- case CK_OpteronSSE3:
- case CK_Athlon64:
- case CK_Athlon64SSE3:
- case CK_AthlonFX:
defineCPUMacros(Builder, "k8");
break;
case CK_AMDFAM10:
@@ -1553,55 +1533,37 @@
.Case("i686", CK_i686)
.Case("pentiumpro", CK_PentiumPro)
.Case("pentium2", CK_Pentium2)
- .Case("pentium3", CK_Pentium3)
- .Case("pentium3m", CK_Pentium3M)
+ .Cases("pentium3", "pentium3m", CK_Pentium3)
.Case("pentium-m", CK_PentiumM)
.Case("c3-2", CK_C3_2)
.Case("yonah", CK_Yonah)
- .Case("pentium4", CK_Pentium4)
- .Case("pentium4m", CK_Pentium4M)
+ .Cases("pentium4", "pentium4m", CK_Pentium4)
.Case("prescott", CK_Prescott)
.Case("nocona", CK_Nocona)
.Case("core2", CK_Core2)
.Case("penryn", CK_Penryn)
- .Case("bonnell", CK_Bonnell)
- .Case("atom", CK_Bonnell) // Legacy name.
- .Case("silvermont", CK_Silvermont)
- .Case("slm", CK_Silvermont) // Legacy name.
+ .Cases("bonnell", "atom", CK_Bonnell)
+ .Cases("silvermont", "slm", CK_Silvermont)
.Case("goldmont", CK_Goldmont)
- .Case("nehalem", CK_Nehalem)
- .Case("corei7", CK_Nehalem) // Legacy name.
+ .Cases("nehalem", "corei7", CK_Nehalem)
.Case("westmere", CK_Westmere)
- .Case("sandybridge", CK_SandyBridge)
- .Case("corei7-avx", CK_SandyBridge) // Legacy name.
- .Case("ivybridge", CK_IvyBridge)
- .Case("core-avx-i", CK_IvyBridge) // Legacy name.
- .Case("haswell", CK_Haswell)
- .Case("core-avx2", CK_Haswell) // Legacy name.
+ .Cases("sandybridge", "corei7-avx", CK_SandyBridge)
+ .Cases("ivybridge", "core-avx-i", CK_IvyBridge)
+ .Cases("haswell", "core-avx2", CK_Haswell)
.Case("broadwell", CK_Broadwell)
.Case("skylake", CK_SkylakeClient)
- .Case("skylake-avx512", CK_SkylakeServer)
- .Case("skx", CK_SkylakeServer) // Legacy name.
+ .Cases("skylake-avx512", "skx", CK_SkylakeServer)
.Case("cannonlake", CK_Cannonlake)
.Case("knl", CK_KNL)
.Case("lakemont", CK_Lakemont)
.Case("k6", CK_K6)
.Case("k6-2", CK_K6_2)
Re: [PATCH] D33514: [WIP] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled
Thanks for doing this! It has been long time I work on big change for LLVM so sorry for forgetting that. On Saturday, September 16, 2017, NAKAMURA Takumi via Phabricator < revi...@reviews.llvm.org> wrote: > chapuni added a comment. > > Fixed in https://reviews.llvm.org/rL313456. > > > > > Comment at: include/llvm/IR/DiagnosticHandler.h:12 > +//===-- > ===// > + > +#include "llvm/ADT/StringRef.h" > > Did you forget include guard here? > > > https://reviews.llvm.org/D33514 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r313470 - Fix a typo in the documentation. NFC.
Author: mstorsjo Date: Sat Sep 16 13:52:05 2017 New Revision: 313470 URL: http://llvm.org/viewvc/llvm-project?rev=313470&view=rev Log: Fix a typo in the documentation. NFC. Modified: libunwind/trunk/docs/index.rst Modified: libunwind/trunk/docs/index.rst URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/docs/index.rst?rev=313470&r1=313469&r2=313470&view=diff == --- libunwind/trunk/docs/index.rst (original) +++ libunwind/trunk/docs/index.rst Sat Sep 16 13:52:05 2017 @@ -15,7 +15,7 @@ that never materialized (e.g. remote unw The unwinder has two levels of API. The high level APIs are the `_Unwind_*` functions which implement functionality required by `__cxa_*` exception -funcionts. The low level APIs are the `unw_*` functions which are an interface +functions. The low level APIs are the `unw_*` functions which are an interface defined by the old HP libunwind project. Getting Started with libunwind ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"
rjmccall added a comment. Thanks, that looks good. https://reviews.llvm.org/D32210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver
compnerd added inline comments.
Comment at: lib/CodeGen/CodeGenModule.cpp:477
Context.getTypeSizeInChars(Context.getWideCharType()).getQuantity();
- assert((LangOpts.ShortWChar ||
- llvm::TargetLibraryInfoImpl::getTargetWCharSize(Target.getTriple())
==
compnerd wrote:
> MatzeB wrote:
> > rnk wrote:
> > > @MatzeB ptal
> > Can you find a new place for this assert()? Please do not just remove it!
> >
> > For the backstory: Unfortunately I had to duplicate the wchar decision
> > logic inside llvm (TargetLibraryInfoImpl::getTargetWCharSize() for cases
> > where we just have the target triple available but need to know the size of
> > wchar_t using library function. This means the logic in LLVM needs to be
> > updated when support for new platforms is added but for people adding
> > platform support it will not be obvious that they have the change
> > LLVM/TargetLibraryInfo as well unless an assert() point them to there being
> > a mismatch.
> Sure, I'll try to see if I can find a suitable place or adjustment of it.
> However, one thing to note is that the frontend does actually embed that into
> the IR metadata ("wchar_size").
I think that if we try to retain this assertion, we need to update all the
tests to ensure that they pass the arguments for selecting the `wchar_t` type.
The entire problem is that the backend view of this cannot correlate with what
the user specified without passing it back to it. The "wchar_size" metadata
does exactly that. Using that to perform the validation for the library
function call.
Repository:
rL LLVM
https://reviews.llvm.org/D37891
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check
lebedev.ri updated this revision to Diff 115547.
lebedev.ri marked 13 inline comments as done.
lebedev.ri added a comment.
Address even more @JonasToth's review notes.
- Fixed docs
- Validated the implementation (the testcase) against the "reference"
implementation
- Fixed discrepancies in the implementation, except the cases which are not
implemented in the "reference" implementation - nested methods and such
I will double-check, but i believe all the current tests are identical
(produce the same metric) right now.
- Added a bit more tests
- Code cleanup
FIXME: so should `const std::array
CognitiveComplexity::Msgs` be `static` and initialized out-of-line, or not
`static`, but initialized in-line?
Repository:
rL LLVM
https://reviews.llvm.org/D36836
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
clang-tidy/readability/FunctionCognitiveComplexityCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
test/clang-tidy/readability-function-cognitive-complexity.cpp
Index: test/clang-tidy/readability-function-cognitive-complexity.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-function-cognitive-complexity.cpp
@@ -0,0 +1,952 @@
+// RUN: %check_clang_tidy %s readability-function-cognitive-complexity %t -- -config='{CheckOptions: [{key: readability-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11
+
+// any function should be checked.
+
+extern int ext_func(int x = 0);
+
+int some_func(int x = 0);
+
+static int some_other_func(int x = 0) {}
+
+template void some_templ_func(T x = 0) {}
+
+class SomeClass {
+public:
+ int *begin(int x = 0);
+ int *end(int x = 0);
+ static int func(int x = 0);
+ template void some_templ_func(T x = 0) {}
+};
+
+// nothing ever decreases cognitive complexity, so we can check all the things
+// in one go. none of the following should increase cognitive complexity:
+void unittest_false() {
+ {};
+ ext_func();
+ some_func();
+ some_other_func();
+ some_templ_func();
+ some_templ_func();
+ SomeClass::func();
+ SomeClass C;
+ C.some_templ_func();
+ C.some_templ_func();
+ C.func();
+ C.end();
+ int i = some_func();
+ i = i;
+ i++;
+ --i;
+ i < 0;
+ int j = 0 ?: 1;
+ auto k = new int;
+ delete k;
+ throw i;
+ {
+throw i;
+ }
+end:
+ return;
+}
+
+#if 1
+#define CC100
+#else
+// this macro has cognitive complexity of 100.
+// it is needed to be able to compare the testcases with the
+// reference Sonar implementation. please place it right after the first
+// CHECK-NOTES in each function
+#define CC100 if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){}if(1){}
+#endif
+
+////
+//-- B1. Increments --//
+////
+// Check that every thing listed in B1 of the specification does indeed //
+// recieve the base increment, and that not-body does not increase nesting//
+////
+
+// break does not increase cognitive complexity.
+// only break LABEL does, but it is unavaliable in C or C++
+
+// continue does not increase cognitive complexity.
+// only continue LABEL does, but it is unavaliable in C or C++
+
+void unittest_b1_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_00' has cognitive complexity of 33 (threshold 0) [readability-function-cognitive-complexity]
+ CC100;
+
+ if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+} else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:18: note: +3, including nesting penalty of 2, nesting level increased to 3{{$}}
+} else {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +1, nesting level increased to 2{{$}}
+}
+ } else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:16: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]
[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check
lebedev.ri added inline comments.
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102-114
+const std::array CognitiveComplexity::Msgs = {{
+// B1 + B2 + B3
+"+%0, including nesting penalty of %1, nesting level increased to %2",
+
+// B1 + B2
+"+%0, nesting level increased to %2",
+
JonasToth wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > could this initialization land in line 45? that would be directly close
> > > to the criteria.
> > It would be nice indeed, but if i do
> > ```
> > // All the possible messages that can be outputed. The choice of the
> > message
> > // to use is based of the combination of the Criterias
> > static constexpr std::array Msgs = {{
> > // B1 + B2 + B3
> > "+%0, including nesting penalty of %1, nesting level increased to %2",
> >
> > // B1 + B2
> > "+%0, nesting level increased to %2",
> >
> > // B1
> > "+%0",
> >
> > // B2
> > "nesting level increased to %2",
> > }};
> > ```
> > i get
> > ```
> > $ ninja check-clang-tools -j1
> > [1/5] Linking CXX executable bin/clang-tidy
> > FAILED: bin/clang-tidy
> > : && /usr/local/bin/clang++ -fPIC -fvisibility-inlines-hidden
> > -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -W
> > -Wno-unused-parameter -Wwrite-strings -Wcast-qual
> > -Wmissing-field-initializers -pedantic -Wno-long-long
> > -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
> > -Wstring-conversion -fcolor-diagnostics -ffunction-sections -fdata-sections
> > -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG
> > -Wl,-allow-shlib-undefined-Wl,-O3 -Wl,--gc-sections
> > tools/clang/tools/extra/clang-tidy/tool/CMakeFiles/clang-tidy.dir/ClangTidyMain.cpp.o
> > -o bin/clang-tidy -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a
> > -lpthread lib/libclangAST.a lib/libclangASTMatchers.a lib/libclangBasic.a
> > lib/libclangTidy.a lib/libclangTidyAndroidModule.a
> > lib/libclangTidyBoostModule.a lib/libclangTidyBugproneModule.a
> > lib/libclangTidyCERTModule.a lib/libclangTidyCppCoreGuidelinesModule.a
> > lib/libclangTidyGoogleModule.a lib/libclangTidyHICPPModule.a
> > lib/libclangTidyLLVMModule.a lib/libclangTidyMiscModule.a
> > lib/libclangTidyModernizeModule.a lib/libclangTidyMPIModule.a
> > lib/libclangTidyPerformanceModule.a lib/libclangTidyReadabilityModule.a
> > lib/libclangTooling.a lib/libclangToolingCore.a
> > lib/libclangTidyCppCoreGuidelinesModule.a lib/libclangTidyGoogleModule.a
> > lib/libclangTidyMiscModule.a lib/libclangTidyReadabilityModule.a
> > lib/libclangTidyUtils.a lib/libclangTidy.a lib/libclangTooling.a
> > lib/libclangFormat.a lib/libclangToolingCore.a
> > lib/libclangStaticAnalyzerFrontend.a lib/libclangFrontend.a
> > lib/libclangDriver.a lib/libLLVMOption.a lib/libclangParse.a
> > lib/libLLVMMCParser.a lib/libclangSerialization.a lib/libclangSema.a
> > lib/libclangEdit.a lib/libLLVMBitReader.a lib/libLLVMProfileData.a
> > lib/libclangStaticAnalyzerCheckers.a lib/libclangStaticAnalyzerCore.a
> > lib/libclangASTMatchers.a lib/libclangRewrite.a lib/libclangAnalysis.a
> > lib/libclangAST.a lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a
> > lib/libLLVMBinaryFormat.a lib/libLLVMMC.a lib/libLLVMSupport.a -lrt -ldl
> > -ltinfo -lpthread -lz -lm lib/libLLVMDemangle.a && :
> > lib/libclangTidyReadabilityModule.a(FunctionCognitiveComplexityCheck.cpp.o):FunctionCognitiveComplexityCheck.cpp:function
> >
> > clang::tidy::readability::FunctionCognitiveComplexityCheck::check(clang::ast_matchers::MatchFinder::MatchResult
> > const&): error: undefined reference to
> > 'clang::tidy::readability::(anonymous namespace)::CognitiveComplexity::Msgs'
> > ```
> > Same if `process()` returns `std::pair` and in
> > `FunctionCognitiveComplexityCheck::check()` i do `const char
> > *IncreaseMessage = Visitor.CC.Msgs[IncreaseMsgId];`
> might the `static` cause that linking error?
> Did you consider using `StringRef` instead of `const char*`? It is better
> behaved.
>
> ```constexpr std::array Msgs = { /* messages */ };```
Actually, found a combination that works, done.
FIXME: so should `const std::array
CognitiveComplexity::Msgs` be `static` and initialized out-of-line, or not
`static`, but initialized in-line?
Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:228-234
+// if increases nesting level
+++CurrentNestingLevel;
+ShouldContinue = TraverseStmt(Node->getThen());
+--CurrentNestingLevel;
+
+if (!ShouldContinue || !Node->getElse())
+ return ShouldContinue;
JonasToth wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > line 228-234 are a pattern, existing in all traversals. i think it could
> > > be refactored into its own function with a descriptive name. Especially
> > > the increment, traverse, decrement isn't obvious immediatly, but
Re: r313315 - Diagnostic specific failed condition in a static_assert.
This is a bug in the libc++ tests. It's OK for them to check that the
static_assert message is produced, but not that they're prefixed with the
exact string "static_assert failed:".
On 16 September 2017 at 05:54, NAKAMURA Takumi via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> This triggered failure in libcxx tests.
> http://bb.pgr.jp/builders/bootstrap-clang-libcxx-lld-i686-linux/builds/97
>
>
> On Fri, Sep 15, 2017 at 8:40 AM Douglas Gregor via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: dgregor
>> Date: Thu Sep 14 16:38:42 2017
>> New Revision: 313315
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=313315&view=rev
>> Log:
>> Diagnostic specific failed condition in a static_assert.
>>
>> When a static_assert fails, dig out a specific condition to diagnose,
>> using the same logic that we use to find the enable_if condition to
>> diagnose.
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaTemplate.cpp
>> cfe/trunk/test/SemaCXX/static-assert.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
>> DiagnosticSemaKinds.td?rev=313315&r1=313314&r2=313315&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 14
>> 16:38:42 2017
>> @@ -1219,6 +1219,8 @@ def warn_messaging_unqualified_id : Warn
>> def err_static_assert_expression_is_not_constant : Error<
>>"static_assert expression is not an integral constant expression">;
>> def err_static_assert_failed : Error<"static_assert failed%select{
>> %1|}0">;
>> +def err_static_assert_requirement_failed : Error<
>> + "static_assert failed due to requirement '%0'%select{ %2|}1">;
>> def ext_static_assert_no_message : ExtWarn<
>>"static_assert with no message is a C++17 extension">, InGroup;
>> def warn_cxx14_compat_static_assert_no_message : Warning<
>>
>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
>> clang/Sema/Sema.h?rev=313315&r1=313314&r2=313315&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 14 16:38:42 2017
>> @@ -2783,6 +2783,14 @@ public:
>>EnableIfAttr *CheckEnableIf(FunctionDecl *Function, ArrayRef
>> Args,
>>bool MissingImplicitThis = false);
>>
>> + /// Find the failed Boolean condition within a given Boolean
>> + /// constant expression, and describe it with a string.
>> + ///
>> + /// \param AllowTopLevelCond Whether to allow the result to be the
>> + /// complete top-level condition.
>> + std::pair
>> + findFailedBooleanCondition(Expr *Cond, bool AllowTopLevelCond);
>> +
>>/// Emit diagnostics for the diagnose_if attributes on Function,
>> ignoring any
>>/// non-ArgDependent DiagnoseIfAttrs.
>>///
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
>> SemaDeclCXX.cpp?rev=313315&r1=313314&r2=313315&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 14 16:38:42 2017
>> @@ -13296,8 +13296,20 @@ Decl *Sema::BuildStaticAssertDeclaration
>>llvm::raw_svector_ostream Msg(MsgBuffer);
>>if (AssertMessage)
>> AssertMessage->printPretty(Msg, nullptr, getPrintingPolicy());
>> - Diag(StaticAssertLoc, diag::err_static_assert_failed)
>> -<< !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
>> +
>> + Expr *InnerCond = nullptr;
>> + std::string InnerCondDescription;
>> + std::tie(InnerCond, InnerCondDescription) =
>> +findFailedBooleanCondition(Converted.get(),
>> + /*AllowTopLevelCond=*/false);
>> + if (InnerCond) {
>> +Diag(StaticAssertLoc, diag::err_static_assert_
>> requirement_failed)
>> + << InnerCondDescription << !AssertMessage
>> + << Msg.str() << InnerCond->getSourceRange();
>> + } else {
>> +Diag(StaticAssertLoc, diag::err_static_assert_failed)
>> + << !AssertMessage << Msg.str() << AssertExpr->getSourceRange();
>> + }
>>Failed = true;
>> }
>>}
>>
>> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
>> SemaTemplate.cpp?rev=313315&r1=313314&r2=313315&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original
[PATCH] D37954: Expand absolute system header paths when using -MD depfiles
Lekensteyn created this revision.
Herald added a subscriber: klimek.
GCC defaults to expanding the dependencies in depfiles, resolving
components like ".." and symlinks. Mimic this feature to ensure that the
Ninja build tool detects the correct dependencies when a symlink changes
directory levels, see https://github.com/ninja-build/ninja/issues/1330
An option to disable this feature is added in case "these changed header
paths may conflict with some compilation environments", see
https://gcc.gnu.org/ml/gcc-patches/2012-09/msg00287.html
Note that under GCC, this option would also affect the paths shown in
the preprocessed output (-E), but that is not implemented here since I
am not sure if it is useful or breaks something else.
https://reviews.llvm.org/D37954
Files:
include/clang/Driver/Options.td
include/clang/Frontend/DependencyOutputOptions.h
lib/Driver/Job.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/DependencyFile.cpp
lib/Tooling/ArgumentsAdjusters.cpp
test/Preprocessor/dependencies-realpath.c
Index: test/Preprocessor/dependencies-realpath.c
===
--- /dev/null
+++ test/Preprocessor/dependencies-realpath.c
@@ -0,0 +1,32 @@
+// RUN: mkdir -p %T/sub/dir
+// RUN: echo > %T/sub/empty.h
+
+// Test that absolute system header paths are expanded in -MD
+//
+// RUN: %clang -fsyntax-only -MD -MF %t.d -MT foo %s -isystem %T/sub/dir/..
+// RUN: FileCheck -check-prefix=TEST1 %s < %t.d
+// TEST1: foo:
+// TEST1: sub{{/|\\}}empty.h
+
+// Test that relative system header paths are not expanded in -MD
+//
+// RUN: cd %T && %clang -fsyntax-only -MD -MF %t.d -MT foo %s -isystem sub/dir/..
+// RUN: FileCheck -check-prefix=TEST2 %s < %t.d
+// TEST2: foo:
+// TEST2: sub/dir/..{{/|\\}}empty.h
+
+// Test that absolute user header paths are not expanded in -MD
+//
+// RUN: cd %T && %clang -fsyntax-only -MD -MF %t.d -MT foo %s -I %T/sub/dir/..
+// RUN: FileCheck -check-prefix=TEST3 %s < %t.d
+// TEST3: foo:
+// TEST3: sub/dir/..{{/|\\}}empty.h
+
+// Test that absolute system header paths are not expanded in -MD with -fno-canonical-system-headers
+//
+// RUN: cd %T && %clang -fsyntax-only -MD -MF %t.d -MT foo %s -I %T/sub/dir/.. -fno-canonical-system-headers
+// RUN: FileCheck -check-prefix=TEST4 %s < %t.d
+// TEST4: foo:
+// TEST4: sub/dir/..{{/|\\}}empty.h
+
+#include
Index: lib/Tooling/ArgumentsAdjusters.cpp
===
--- lib/Tooling/ArgumentsAdjusters.cpp
+++ lib/Tooling/ArgumentsAdjusters.cpp
@@ -58,7 +58,8 @@
StringRef Arg = Args[i];
// All dependency-file options begin with -M. These include -MM,
// -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
- if (!Arg.startswith("-M"))
+ // The exception is -fno-canonical-system-headers.
+ if (!Arg.startswith("-M") && !Arg.equals("-fno-canonical-system-headers"))
AdjustedArgs.push_back(Args[i]);
if ((Arg == "-MF") || (Arg == "-MT") || (Arg == "-MQ") ||
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -161,6 +161,7 @@
bool AddMissingHeaderDeps;
bool SeenMissingHeader;
bool IncludeModuleFiles;
+ bool CanonicalSystemHeaders;
DependencyOutputFormat OutputFormat;
private:
@@ -176,6 +177,7 @@
AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
SeenMissingHeader(false),
IncludeModuleFiles(Opts.IncludeModuleFiles),
+ CanonicalSystemHeaders(Opts.CanonicalSystemHeaders),
OutputFormat(Opts.OutputFormat) {
for (const auto &ExtraDep : Opts.ExtraDeps) {
AddFilename(ExtraDep);
@@ -288,6 +290,16 @@
if (!FileMatchesDepCriteria(Filename.data(), FileType))
return;
+ // Canonicalize absolute system header paths like GCC does (unless
+ // -fno-canonical-system-headers is given).
+ if (CanonicalSystemHeaders && isSystem(FileType) &&
+ llvm::sys::path::is_absolute(Filename)) {
+StringRef RealPath = FE->tryGetRealPathName();
+if (!RealPath.empty()) {
+ Filename = RealPath;
+}
+ }
+
AddFilename(llvm::sys::path::remove_leading_dotslash(Filename));
}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -998,6 +998,7 @@
Opts.Targets = Args.getAllArgValues(OPT_MT);
Opts.IncludeSystemHeaders = Args.hasArg(OPT_sys_header_deps);
Opts.IncludeModuleFiles = Args.hasArg(OPT_module_file_deps);
+ Opts.CanonicalSystemHeaders = !Args.hasArg(OPT_fno_canonical_system_headers);
Opts.UsePhonyTargets = Args.hasArg(OPT_MP);
Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
Opts.HeaderIncludeOutputFile = Args.getLastArgValue(OPT_header_include_file);
Index: lib/Driver/ToolChains/Clang.cpp
[PATCH] D37954: Expand absolute system header paths when using -MD depfiles
Lekensteyn added a comment. I tried to contact Simon (the author of the GCC) patch with a question about the -fno-canonical-system-headers, but his Google address is bouncing. GCC has long survived with doing this by default, I wonder if this option could just me omitted, enabling the feature by default. https://reviews.llvm.org/D37954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37955: [libcxx] Fix invert negative bracket match.
timshen created this revision.
Herald added a subscriber: sanjoy.
Herald added a reviewer: EricWF.
Ideally we want !(neg_mask || neg_char), aka (!neg_mask && !neg_char). Before
the change, the code is (!neg_mask || !neg_char).
This fixes PR34310.
https://reviews.llvm.org/D37955
Files:
libcxx/include/regex
libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
@@ -0,0 +1,30 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// template
+// bool
+// regex_search(BidirectionalIterator first, BidirectionalIterator last,
+// match_results& m,
+// const basic_regex& e,
+// regex_constants::match_flag_type flags =
regex_constants::match_default);
+
+#include
+#include
+#include
+#include "test_macros.h"
+
+// PR34310
+int main()
+{
+ assert(std::regex_search("HelloWorld", std::regex("[^\\W]")));
+ assert(std::regex_search("_", std::regex("[^\\W]")));
+ return 0;
+}
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -2409,17 +2409,23 @@
goto __exit;
}
}
-if (!__neg_chars_.empty())
+// set of "__found" chars =
+// union(complement(union(__neg_chars_, __neg_mask_)),
+// other cases...)
+// __neg_chars_ and __neg_mask_'d better be handled together, as there
+// are no short circuit opportunities.
{
-for (size_t __i = 0; __i < __neg_chars_.size(); ++__i)
-{
-if (__ch == __neg_chars_[__i])
-goto __is_neg_char;
-}
+ const bool __in_neg_mask = __neg_mask_ &&
+ __traits_.isctype(__ch, __neg_mask_);
+ const bool __in_neg_chars =
+ std::find(__neg_chars_.begin(), __neg_chars_.end(), __ch) !=
+ __neg_chars_.end();
+ if (!(__in_neg_mask || __in_neg_chars))
+ {
__found = true;
goto __exit;
+ }
}
-__is_neg_char:
if (!__ranges_.empty())
{
string_type __s2 = __collate_ ?
@@ -2451,11 +2457,6 @@
__found = true;
goto __exit;
}
-if (__neg_mask_ && !__traits_.isctype(__ch, __neg_mask_))
-{
-__found = true;
-goto __exit;
-}
}
else
__found = __negate_; // force reject
Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
@@ -0,0 +1,30 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// template
+// bool
+// regex_search(BidirectionalIterator first, BidirectionalIterator last,
+// match_results& m,
+// const basic_regex& e,
+// regex_constants::match_flag_type flags = regex_constants::match_default);
+
+#include
+#include
+#include
+#include "test_macros.h"
+
+// PR34310
+int main()
+{
+ assert(std::regex_search("HelloWorld", std::regex("[^\\W]")));
+ assert(std::regex_search("_", std::regex("[^\\W]")));
+ return 0;
+}
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -2409,17 +2409,23 @@
goto __exit;
}
}
-if (!__neg_chars_.empty())
+// set of "__found" chars =
+// union(complement(union(__neg_chars_, __neg_mask_)),
+// other cases...)
+// __neg_chars_ and __neg_mask_'d better be handled together, as there
+// are no short circuit opportunities.
{
-for (size_t __i = 0; __i < __neg_chars_.size(); ++__i)
-{
-if (__ch == __neg_chars_[__i])
-goto __is_neg_char;
-}
+ const bool __in_neg_mask = __neg_mask_ &&
+ __trai
[PATCH] D37955: [libcxx] Fix invert negative bracket match.
timshen updated this revision to Diff 115554.
timshen added a comment.
Remove "#include " in the test. It was for debugging.
https://reviews.llvm.org/D37955
Files:
libcxx/include/regex
libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
@@ -0,0 +1,29 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// template
+// bool
+// regex_search(BidirectionalIterator first, BidirectionalIterator last,
+// match_results& m,
+// const basic_regex& e,
+// regex_constants::match_flag_type flags =
regex_constants::match_default);
+
+#include
+#include
+#include "test_macros.h"
+
+// PR34310
+int main()
+{
+ assert(std::regex_search("HelloWorld", std::regex("[^\\W]")));
+ assert(std::regex_search("_", std::regex("[^\\W]")));
+ return 0;
+}
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -2409,17 +2409,23 @@
goto __exit;
}
}
-if (!__neg_chars_.empty())
+// set of "__found" chars =
+// union(complement(union(__neg_chars_, __neg_mask_)),
+// other cases...)
+// __neg_chars_ and __neg_mask_'d better be handled together, as there
+// are no short circuit opportunities.
{
-for (size_t __i = 0; __i < __neg_chars_.size(); ++__i)
-{
-if (__ch == __neg_chars_[__i])
-goto __is_neg_char;
-}
+ const bool __in_neg_mask = __neg_mask_ &&
+ __traits_.isctype(__ch, __neg_mask_);
+ const bool __in_neg_chars =
+ std::find(__neg_chars_.begin(), __neg_chars_.end(), __ch) !=
+ __neg_chars_.end();
+ if (!(__in_neg_mask || __in_neg_chars))
+ {
__found = true;
goto __exit;
+ }
}
-__is_neg_char:
if (!__ranges_.empty())
{
string_type __s2 = __collate_ ?
@@ -2451,11 +2457,6 @@
__found = true;
goto __exit;
}
-if (__neg_mask_ && !__traits_.isctype(__ch, __neg_mask_))
-{
-__found = true;
-goto __exit;
-}
}
else
__found = __negate_; // force reject
Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
@@ -0,0 +1,29 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// template
+// bool
+// regex_search(BidirectionalIterator first, BidirectionalIterator last,
+// match_results& m,
+// const basic_regex& e,
+// regex_constants::match_flag_type flags = regex_constants::match_default);
+
+#include
+#include
+#include "test_macros.h"
+
+// PR34310
+int main()
+{
+ assert(std::regex_search("HelloWorld", std::regex("[^\\W]")));
+ assert(std::regex_search("_", std::regex("[^\\W]")));
+ return 0;
+}
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -2409,17 +2409,23 @@
goto __exit;
}
}
-if (!__neg_chars_.empty())
+// set of "__found" chars =
+// union(complement(union(__neg_chars_, __neg_mask_)),
+// other cases...)
+// __neg_chars_ and __neg_mask_'d better be handled together, as there
+// are no short circuit opportunities.
{
-for (size_t __i = 0; __i < __neg_chars_.size(); ++__i)
-{
-if (__ch == __neg_chars_[__i])
-goto __is_neg_char;
-}
+ const bool __in_neg_mask = __neg_mask_ &&
+ __traits_.isctype(__ch, __neg_mask_);
+ const bool __in_neg_chars =
+ std::find(__neg_chars_.begin(), __neg_chars_.end(), __ch) !=
[PATCH] D37956: [libc++] Check back reference subscript overflow in a single place.
timshen created this revision.
Herald added a subscriber: sanjoy.
Herald added a reviewer: EricWF.
Currently it's checked in three places, including one place that's at
regex runtime.
There is no need to do runtime check, as all uses and definitions of
subexpress is known at regex compile-time.
This fixes PR34297.
https://reviews.llvm.org/D37956
Files:
libcxx/include/regex
libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
Index: libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
===
--- libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
+++ libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
@@ -19,11 +19,14 @@
#include
#include "test_macros.h"
-static bool error_badbackref_thrown(const char *pat)
+static bool error_badbackref_thrown(
+const char *pat,
+std::regex_constants::syntax_option_type flags =
+std::regex_constants::ECMAScript)
{
bool result = false;
try {
-std::regex re(pat);
+std::regex re(pat, flags);
} catch (const std::regex_error &ex) {
result = (ex.code() == std::regex_constants::error_backref);
}
@@ -34,6 +37,7 @@
{
assert(error_badbackref_thrown("\\1abc")); // no references
assert(error_badbackref_thrown("ab(c)\\2def")); // only one reference
+assert(error_badbackref_thrown("(cat)\\1", std::regex_constants::basic));
// this should NOT throw, because we only should look at the '1'
// See https://bugs.llvm.org/show_bug.cgi?id=31387
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -1745,8 +1745,6 @@
void
__back_ref<_CharT>::__exec(__state& __s) const
{
-if (__mexp_ > __s.__sub_matches_.size())
-__throw_regex_error();
sub_match& __sm = __s.__sub_matches_[__mexp_-1];
if (__sm.matched)
{
@@ -4320,8 +4318,6 @@
for (++__first;
__first != __last && '0' <= *__first && *__first <= '9';
++__first)
__v = 10 * __v + *__first - '0';
-if (__v > mark_count())
-__throw_regex_error();
__push_back_ref(__v);
}
}
@@ -4712,6 +4708,9 @@
void
basic_regex<_CharT, _Traits>::__push_back_ref(int __i)
{
+if (__i > static_cast(mark_count()))
+__throw_regex_error();
+
if (flags() & icase)
__end_->first() = new __back_ref_icase<_CharT, _Traits>
(__traits_, __i,
__end_->first());
Index: libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
===
--- libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
+++ libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
@@ -19,11 +19,14 @@
#include
#include "test_macros.h"
-static bool error_badbackref_thrown(const char *pat)
+static bool error_badbackref_thrown(
+const char *pat,
+std::regex_constants::syntax_option_type flags =
+std::regex_constants::ECMAScript)
{
bool result = false;
try {
-std::regex re(pat);
+std::regex re(pat, flags);
} catch (const std::regex_error &ex) {
result = (ex.code() == std::regex_constants::error_backref);
}
@@ -34,6 +37,7 @@
{
assert(error_badbackref_thrown("\\1abc")); // no references
assert(error_badbackref_thrown("ab(c)\\2def")); // only one reference
+assert(error_badbackref_thrown("(cat)\\1", std::regex_constants::basic));
// this should NOT throw, because we only should look at the '1'
// See https://bugs.llvm.org/show_bug.cgi?id=31387
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -1745,8 +1745,6 @@
void
__back_ref<_CharT>::__exec(__state& __s) const
{
-if (__mexp_ > __s.__sub_matches_.size())
-__throw_regex_error();
sub_match& __sm = __s.__sub_matches_[__mexp_-1];
if (__sm.matched)
{
@@ -4320,8 +4318,6 @@
for (++__first;
__first != __last && '0' <= *__first && *__first <= '9'; ++__first)
__v = 10 * __v + *__first - '0';
-if (__v > mark_count())
-__throw_regex_error();
__push_back_ref(__v);
}
}
@@ -4712,6 +4708,9 @@
void
basic_regex<_CharT, _Traits>::__push_back_ref(int __i)
{
+if (__i > static_cast(mark_count()))
+__throw_regex_error();
+
if (flags() & icase)
__end_->first() = new __back_ref_icase<_CharT, _Traits>
(__traits_, __i, __end_->first());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co
