alexfh added a comment.
Too much noise here as well. Particularly, `someBool == true` should not be
changed to `someBool == 1`. Please fix the check and update the results.
================
Comment at: clang-tidy/readability/ImplicitBoolCastCheck.cpp:253
@@ -252,3 +252,3 @@
DestinationType->isMemberPointerType()) &&
- BoolLiteralExpression->getValue() == false) {
+ BoolLiteralExpression->getValue() == 0) {
return "0";
----------------
Seems like a bug. `CXXBoolLiteralExpr::getValue()` returns `bool`.
================
Comment at: lib/AsmParser/LLParser.cpp:4868
@@ -4867,3 +4867,3 @@
///
int LLParser::ParseInstruction(Instruction *&Inst, BasicBlock *BB,
PerFunctionState &PFS) {
----------------
Looks like this should return `InstResult` and the return values should be
replaced by proper enumerators.
================
Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:1562
@@ -1563,1 +1561,3 @@
+ IsBottomUp = 1,
+ HasReadyFilter = 0
};
----------------
Quuxplusone wrote:
> If I understand correctly, the idiomatic C++11 way to write this code would be
>
> static constexpr bool IsBottomUp = true;
> static constexpr bool HasReadyFilter = false;
>
> I think the proposed correction is worse than the original, because it makes
> this look like an integer enumeration.
I agree that changing this to integers is a regression. However, we can't yet
use `constexpr`, since VS2013 doesn't support it. Looks like this file just
needs to be reverted.
================
Comment at: lib/IR/Core.cpp:227
@@ -226,3 +226,3 @@
*ErrorMessage = strdup(EC.message().c_str());
- return true;
+ return 1;
}
----------------
Quuxplusone wrote:
> This correction is wrong; either `true` should be accepted as a valid value
> for `LLVMBool`, or `LLVMBool` should be replaced with `bool`.
This function is a part of C API, so we can't use `bool` here, and `LLVMBool`
is the right choice. However, it might make sense to add `LLVM_TRUE` and
`LLVM_FALSE` constants/macros to `include/llvm-c/Types.h` to more clearly
specify possible values for the `LLVMBool` type.
================
Comment at: lib/Lex/PPMacroExpansion.cpp:1689
@@ -1688,3 +1688,3 @@
if (!II)
- return false;
+ return 0;
else if (II->getBuiltinID() != 0)
----------------
This actually seems like a useful replacement ;)
================
Comment at: lib/MC/MCContext.cpp:314
@@ -313,3 +313,3 @@
MCSectionELF(I->getKey(), Type, Flags, SectionKind::getReadOnly(),
- EntrySize, Group, true, nullptr, Associated);
+ EntrySize, Group, 1, nullptr, Associated);
}
----------------
That one is interesting: passing `true` (or `1`) to `unsigned UniqueID` seems
like a very weird thing to do. Please at least add a `/*UniqueID=*/` comment
before the argument to make this weirdness more explicit.
================
Comment at: lib/Sema/SemaOpenMP.cpp:4487
@@ -4486,3 +4486,3 @@
// OK to convert to signed, because new type has more bits than old.
- QualType NewType = C.getIntTypeForBitwidth(Bits, /* Signed */ true);
+ QualType NewType = C.getIntTypeForBitwidth(Bits, /* Signed */ 1);
return SemaRef.PerformImplicitConversion(E, NewType, Sema::AA_Converting,
----------------
Instead, the parameter type should be changed to bool.
================
Comment at: lib/Serialization/ASTReader.cpp:5453
@@ -5452,3 +5452,3 @@
AutoTypeKeyword Keyword = (AutoTypeKeyword)Record[Idx++];
- bool IsDependent = Deduced.isNull() ? Record[Idx++] : false;
+ bool IsDependent = Deduced.isNull() ? Record[Idx++] : 0;
return Context.getAutoType(Deduced, Keyword, IsDependent);
----------------
That's wrong. Instead, `Record[Idx++]` should be changed to `Record[Idx++] !=
0`.
================
Comment at: lib/Target/Hexagon/MCTargetDesc/HexagonMCShuffler.cpp:194
@@ -193,3 +193,3 @@
- if (doneShuffling == false) {
+ if (doneShuffling == 0) {
HexagonMCShuffler MCS(MCII, STI, MCB);
----------------
This is wrong.
http://reviews.llvm.org/D19105
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
