[linux-port] Correct conditional groupings (#1326)

Clang and gcc complain when AND and OR logical operations are both
used in a single conditional because of the possibility of mistakes
when depending solely on operator precedence. In fact, this change
fixes a few clear errors mostly in asserts that would never fire.
Fixes 13 clang and 22 gcc warnings.

Additionally, when two if statements are followed by an else, there
may be some ambiguity in terms of which if the else applies to. It
is the closest one, the second in the case of two, but it is another
area of potential mistakes. Adding braces around the code intended
as the block of the first if conditional clarifies this.
fixed 8 clang and 8 gcc warnings.

Clang complains sometimes when assignments take place in a
conditional without additional parenthesis around it. It can
indicate when what was meant to be an equal comparison is replaced
with an assignment.
fixes 2 clang warnings.
This commit is contained in:
Greg Roth 2018-06-02 02:19:14 -06:00 коммит произвёл Lei Zhang
Родитель 4aa0f706c8
Коммит d975f150b7
19 изменённых файлов: 39 добавлений и 31 удалений

Просмотреть файл

@ -19,7 +19,7 @@ namespace dxc {
static void TrimEOL(_Inout_z_ char *pMsg) {
char *pEnd = pMsg + strlen(pMsg);
--pEnd;
while (pEnd > pMsg && *pEnd == '\r' || *pEnd == '\n') {
while (pEnd > pMsg && (*pEnd == '\r' || *pEnd == '\n')) {
--pEnd;
}
pEnd[1] = '\0';

Просмотреть файл

@ -425,7 +425,7 @@ bool OP::IsDxilOpWave(OpCode C) {
// WaveReadLaneFirst=118, WaveActiveOp=119, WaveActiveBit=120,
// WavePrefixOp=121, QuadReadLaneAt=122, QuadOp=123, WaveAllBitCount=135,
// WavePrefixBitCount=136
return 110 <= op && op <= 123 || 135 <= op && op <= 136;
return (110 <= op && op <= 123) || (135 <= op && op <= 136);
// OPCODE-WAVE:END
}
@ -436,7 +436,7 @@ bool OP::IsDxilOpGradient(OpCode C) {
// Instructions: Sample=60, SampleBias=61, SampleCmp=64, TextureGather=73,
// TextureGatherCmp=74, CalculateLOD=81, DerivCoarseX=83, DerivCoarseY=84,
// DerivFineX=85, DerivFineY=86
return 60 <= op && op <= 61 || op == 64 || 73 <= op && op <= 74 || op == 81 || 83 <= op && op <= 86;
return (60 <= op && op <= 61) || op == 64 || (73 <= op && op <= 74) || op == 81 || (83 <= op && op <= 86);
// OPCODE-GRADIENT:END
}

Просмотреть файл

@ -590,7 +590,7 @@ static bool ValidateOpcodeInProfile(DXIL::OpCode opcode,
// CalculateLOD=81, Discard=82, DerivCoarseX=83, DerivCoarseY=84,
// DerivFineX=85, DerivFineY=86, EvalSnapped=87, EvalSampleIndex=88,
// EvalCentroid=89, SampleIndex=90, Coverage=91, InnerCoverage=92
if (60 <= op && op <= 61 || op == 64 || 76 <= op && op <= 77 || 81 <= op && op <= 92)
if ((60 <= op && op <= 61) || op == 64 || (76 <= op && op <= 77) || (81 <= op && op <= 92))
return (pSM->IsPS());
// Instructions: AttributeAtVertex=137
if (op == 137)
@ -2044,7 +2044,7 @@ static bool IsLLVMInstructionAllowed(llvm::Instruction &I) {
// SExt=35, FPToUI=36, FPToSI=37, UIToFP=38, SIToFP=39, FPTrunc=40, FPExt=41,
// BitCast=44, AddrSpaceCast=45, ICmp=46, FCmp=47, PHI=48, Call=49, Select=50,
// ExtractValue=57
return 1 <= op && op <= 3 || 8 <= op && op <= 29 || 31 <= op && op <= 41 || 44 <= op && op <= 50 || op == 57;
return (1 <= op && op <= 3) || (8 <= op && op <= 29) || (31 <= op && op <= 41) || (44 <= op && op <= 50) || op == 57;
// OPCODE-ALLOWED:END
}

Просмотреть файл

@ -1003,17 +1003,19 @@ unsigned HLModule::FindCastOp(bool fromUnsigned, bool toUnsigned,
return Instruction::FPExt;
}
if (SrcTy->isIntOrIntVectorTy() && DstTy->isFPOrFPVectorTy())
if (SrcTy->isIntOrIntVectorTy() && DstTy->isFPOrFPVectorTy()) {
if (fromUnsigned)
return Instruction::UIToFP;
else
return Instruction::SIToFP;
}
if (SrcTy->isFPOrFPVectorTy() && DstTy->isIntOrIntVectorTy())
if (SrcTy->isFPOrFPVectorTy() && DstTy->isIntOrIntVectorTy()) {
if (toUnsigned)
return Instruction::FPToUI;
else
return Instruction::FPToSI;
}
DXASSERT_NOMSG(0);
return castOp;

Просмотреть файл

@ -193,11 +193,12 @@ void PassManagerBuilder::populateFunctionPassManager(
FPM.add(createCFGSimplificationPass());
// HLSL Change - don't run SROA.
// HLSL uses special SROA added in addHLSLPasses.
if (HLSLHighLevel) // HLSL Change
if (HLSLHighLevel) { // HLSL Change
if (UseNewSROA)
FPM.add(createSROAPass());
else
FPM.add(createScalarReplAggregatesPass());
}
// HLSL Change. FPM.add(createEarlyCSEPass());
FPM.add(createLowerExpectIntrinsicPass());
}
@ -365,11 +366,12 @@ void PassManagerBuilder::populateModulePassManager(
// Break up aggregate allocas, using SSAUpdater.
// HLSL Change - don't run SROA.
// HLSL uses special SROA added in addHLSLPasses.
if (HLSLHighLevel) // HLSL Change
if (HLSLHighLevel) { // HLSL Change
if (UseNewSROA)
MPM.add(createSROAPass(/*RequiresDomTree*/ false));
else
MPM.add(createScalarReplAggregatesPass(-1, false));
}
// HLSL Change. MPM.add(createEarlyCSEPass()); // Catch trivial redundancies
// HLSL Change. MPM.add(createJumpThreadingPass()); // Thread jumps.
MPM.add(createCorrelatedValuePropagationPass()); // Propagate conditionals

Просмотреть файл

@ -2021,8 +2021,8 @@ static Value *LoadVectorOrStructArray(ArrayType *AT, ArrayRef<Value *> NewElts,
Value *EltVal = LoadVectorOrStructArray(EltAT, NewElts, idxList, Builder);
retVal = Builder.CreateInsertValue(retVal, EltVal, i);
} else {
assert(EltTy->isVectorTy() ||
EltTy->isStructTy() && "must be a vector or struct type");
assert((EltTy->isVectorTy() ||
EltTy->isStructTy()) && "must be a vector or struct type");
bool isVectorTy = EltTy->isVectorTy();
Value *retVec = llvm::UndefValue::get(EltTy);
@ -2068,8 +2068,8 @@ static void StoreVectorOrStructArray(ArrayType *AT, Value *val,
if (ArrayType *EltAT = dyn_cast<ArrayType>(EltTy)) {
StoreVectorOrStructArray(EltAT, elt, NewElts, idxList, Builder);
} else {
assert(EltTy->isVectorTy() ||
EltTy->isStructTy() && "must be a vector or struct type");
assert((EltTy->isVectorTy() ||
EltTy->isStructTy()) && "must be a vector or struct type");
bool isVectorTy = EltTy->isVectorTy();
if (isVectorTy) {
for (uint32_t c = 0; c < EltTy->getVectorNumElements(); c++) {

Просмотреть файл

@ -330,9 +330,8 @@ void DeclPrinter::VisitDeclContext(DeclContext *DC, bool Indent) {
continue;
}
if (isa<AccessSpecDecl>(*D)
&& !Policy.LangOpts.HLSL // HLSL Change - no access specifier for hlsl.
) {
if (isa<AccessSpecDecl>(*D))
if (!Policy.LangOpts.HLSL) { // HLSL Change - no access specifier for hlsl.
Indentation -= Policy.Indentation;
this->Indent();
Print(D->getAccess());

Просмотреть файл

@ -3390,7 +3390,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
// If the argument doesn't match, perform a bitcast to coerce it. This
// can happen due to trivial type mismatches.
if (FirstIRArg < IRFuncTy->getNumParams() &&
V->getType() != IRFuncTy->getParamType(FirstIRArg))
V->getType() != IRFuncTy->getParamType(FirstIRArg)) {
// HLSL Change Starts
// Generate AddrSpaceCast for shared memory.
if (V->getType()->isPointerTy())
@ -3399,6 +3399,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
else
// HLSL Change Ends
V = Builder.CreateBitCast(V, IRFuncTy->getParamType(FirstIRArg));
}
IRCallArgs[FirstIRArg] = V;
break;
}

Просмотреть файл

@ -1807,13 +1807,14 @@ CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName,
return Entry;
// Make sure the result is of the correct type.
if (Entry->getType()->getAddressSpace() != Ty->getAddressSpace())
if (Entry->getType()->getAddressSpace() != Ty->getAddressSpace()) {
// HLSL Change Begins
// TODO: do we put address space in type?
if (LangOpts.HLSL) return Entry;
else
// HLSL Change Ends
return llvm::ConstantExpr::getAddrSpaceCast(Entry, Ty);
}
return llvm::ConstantExpr::getBitCast(Entry, Ty);
}

Просмотреть файл

@ -513,7 +513,7 @@ NumericLiteralParser::NumericLiteralParser(StringRef TokSpelling,
// and FP constants (specifically, the 'pp-number' regex), and assumes that
// the byte at "*end" is both valid and not part of the regex. Because of
// this, it doesn't have to check for 'overscan' in various places.
assert(!isPreprocessingNumberBody(*ThisTokEnd) || *ThisTokEnd == '.' || *ThisTokEnd == '#' && "didn't maximally munch?"); // HLSL Change - '.' might be a second '.' for a '1.2.x' literal
assert((!isPreprocessingNumberBody(*ThisTokEnd) || *ThisTokEnd == '.' || *ThisTokEnd == '#') && "didn't maximally munch?"); // HLSL Change - '.' might be a second '.' for a '1.2.x' literal
s = DigitsBegin = ThisTokBegin;
saw_inf = false;

Просмотреть файл

@ -262,7 +262,7 @@ void TokenLexer::ExpandFunctionArguments() {
// If it is not the LHS/RHS of a ## operator, we must pre-expand the
// argument and substitute the expanded tokens into the result. This is
// C99 6.10.3.1p1.
if (PP.PPOpts.get()->ExpandTokPastingArg || !PasteBefore && !PasteAfter) { // HLSL Change
if (PP.PPOpts.get()->ExpandTokPastingArg || (!PasteBefore && !PasteAfter)) { // HLSL Change
const Token *ResultArgToks;
// Only preexpand the argument if it could possibly need it. This

Просмотреть файл

@ -1094,7 +1094,7 @@ HLSLReservedKeyword:
case tok::kw___real: // unary-expression: '__real' cast-expression [GNU]
case tok::kw___imag: { // unary-expression: '__imag' cast-expression [GNU]
// HLSL Change Starts
if (getLangOpts().HLSL && Tok.getKind() == tok::kw___real || Tok.getKind() == tok::kw___imag) {
if (getLangOpts().HLSL && (SavedKind == tok::kw___real || SavedKind == tok::kw___imag)) {
goto HLSLReservedKeyword;
}
// HLSL Change Ends

Просмотреть файл

@ -202,6 +202,7 @@ Parser::ParseSingleDeclarationAfterTemplate(
if (Tok.is(tok::kw_using))
// HLSL Change Starts
{
if (getLangOpts().HLSL) {
Diag(Tok, diag::err_hlsl_reserved_keyword) << "using";
SkipMalformedDecl();
@ -211,6 +212,7 @@ Parser::ParseSingleDeclarationAfterTemplate(
return ParseUsingDirectiveOrDeclaration(Context, TemplateInfo, DeclEnd,
prefixAttrs);
} // HLSL Change - close conditional
} // HLSL Change - close conditional
// Parse the declaration specifiers, stealing any diagnostics from
// the template parameters.

Просмотреть файл

@ -1028,7 +1028,7 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
// We should have either an opening brace or, in a C++ constructor,
// we may have a colon.
if (Tok.isNot(tok::l_brace) &&
(getLangOpts().HLSL && Tok.isNot(tok::colon) || // HLSL Change - for HLSL, only allow ':', not try or =
((getLangOpts().HLSL && Tok.isNot(tok::colon)) || // HLSL Change - for HLSL, only allow ':', not try or =
(!getLangOpts().CPlusPlus ||
(Tok.isNot(tok::colon) && Tok.isNot(tok::kw_try) &&
Tok.isNot(tok::equal))))) {

Просмотреть файл

@ -282,7 +282,7 @@ DeclResultIdMapper::getDeclSpirvInfo(const ValueDecl *decl) const {
}
SpirvEvalInfo DeclResultIdMapper::getDeclEvalInfo(const ValueDecl *decl) {
if (const auto *info = getDeclSpirvInfo(decl))
if (const auto *info = getDeclSpirvInfo(decl)) {
if (info->indexInCTBuffer >= 0) {
// If this is a VarDecl inside a HLSLBufferDecl, we need to do an extra
// OpAccessChain to get the pointer to the variable since we created
@ -303,6 +303,7 @@ SpirvEvalInfo DeclResultIdMapper::getDeclEvalInfo(const ValueDecl *decl) {
} else {
return *info;
}
}
emitFatalError("found unregistered decl", decl->getLocation())
<< decl->getName();

Просмотреть файл

@ -3742,7 +3742,7 @@ void SPIRVEmitter::handleOffsetInMethodCall(const CXXMemberCallExpr *expr,
assert(index < expr->getNumArgs());
*constOffset = *varOffset = 0; // Initialize both first
if (*constOffset = tryToEvaluateAsConst(expr->getArg(index)))
if ((*constOffset = tryToEvaluateAsConst(expr->getArg(index))))
return; // Constant offset
else
*varOffset = doExpr(expr->getArg(index));
@ -5388,7 +5388,7 @@ SpirvEvalInfo SPIRVEmitter::createVectorSplat(const Expr *scalarExpr,
// Try to evaluate the element as constant first. If successful, then we
// can generate constant instructions for this vector splat.
if (scalarVal = tryToEvaluateAsConst(scalarExpr)) {
if ((scalarVal = tryToEvaluateAsConst(scalarExpr))) {
isConstVal = true;
} else {
scalarVal = doExpr(scalarExpr);

Просмотреть файл

@ -1035,7 +1035,7 @@ bool TypeTranslator::isOrContainsNonFpColMajorMatrix(QualType type,
const Decl *decl) const {
const auto isColMajorDecl = [this](const Decl *decl) {
return decl->hasAttr<HLSLColumnMajorAttr>() ||
!decl->hasAttr<HLSLRowMajorAttr>() && !spirvOptions.defaultRowMajor;
(!decl->hasAttr<HLSLRowMajorAttr>() && !spirvOptions.defaultRowMajor);
};
QualType elemType = {};

Просмотреть файл

@ -4975,8 +4975,8 @@ bool HLSLExternalSource::MatchArguments(
// Compare template
if ((AR_TOBJ_UNKNOWN == Template[pIntrinsicArg->uTemplateId]) ||
(AR_TOBJ_SCALAR == Template[pIntrinsicArg->uTemplateId]) &&
(AR_TOBJ_VECTOR == TypeInfoShapeKind || AR_TOBJ_MATRIX == TypeInfoShapeKind)) {
((AR_TOBJ_SCALAR == Template[pIntrinsicArg->uTemplateId]) &&
(AR_TOBJ_VECTOR == TypeInfoShapeKind || AR_TOBJ_MATRIX == TypeInfoShapeKind))) {
Template[pIntrinsicArg->uTemplateId] = TypeInfoShapeKind;
}
else if (AR_TOBJ_SCALAR == TypeInfoShapeKind) {

Просмотреть файл

@ -5133,7 +5133,7 @@ static ExprResult CheckConvertedConstantExpression(Sema &S, Expr *From,
QualType T, APValue &Value,
Sema::CCEKind CCE,
bool RequireInt) {
assert(S.getLangOpts().CPlusPlus11 || S.getLangOpts().HLSLVersion >= 2017 &&
assert((S.getLangOpts().CPlusPlus11 || S.getLangOpts().HLSLVersion >= 2017) &&
"converted constant expression outside C++11");
if (checkPlaceholderForOverload(S, From))
@ -5850,8 +5850,8 @@ Sema::AddOverloadCandidate(FunctionDecl *Function,
AllowExplicit);
}
// HLSL Change Ends
if (Candidate.Conversions[ArgIdx].isInitialized() && Candidate.Conversions[ArgIdx].isBad()
|| Candidate.OutConversions[ArgIdx].isInitialized() && Candidate.OutConversions[ArgIdx].isBad()) { // HLSL Change - add out conversion, check initialized
if ((Candidate.Conversions[ArgIdx].isInitialized() && Candidate.Conversions[ArgIdx].isBad())
|| (Candidate.OutConversions[ArgIdx].isInitialized() && Candidate.OutConversions[ArgIdx].isBad())) { // HLSL Change - add out conversion, check initialized
Candidate.Viable = false;
Candidate.FailureKind = ovl_fail_bad_conversion;
return;