From ee87cc2a1e04457a5e419cb27e0c18f5bd4f2e4f Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Fri, 21 Aug 2015 11:51:28 -0400 Subject: [PATCH] Fix TextAdvance() problems involving whitespace around comment lines. Fix the bug that TextAdvance() forgot to skip whitespace at the beginning of the next line after a comment line. Fix the bug that TextAdvanceLine() increase line number after going over a character. --- CMakeLists.txt | 3 ++- source/text.cpp | 5 +++-- test/Comment.cpp | 47 +++++++++++++++++++++++++++++++++++++++++++++++ test/TextAdvance.cpp | 17 +++++++++++++++++ test/TextToBinary.cpp | 18 +++++++++--------- 5 files changed, 78 insertions(+), 12 deletions(-) create mode 100644 test/Comment.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 710716b..d903f2d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -157,9 +157,10 @@ if (TARGET gtest) ${CMAKE_CURRENT_SOURCE_DIR}/test/UnitSPIRV.h ${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryEndianness.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/test/FixWord.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryHeaderGet.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryToText.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test/Comment.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test/FixWord.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/LibspirvMacros.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/NamedId.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test/OpcodeTableGet.cpp diff --git a/source/text.cpp b/source/text.cpp index f33d828..8fdcdcd 100644 --- a/source/text.cpp +++ b/source/text.cpp @@ -111,7 +111,7 @@ spv_result_t spvTextAdvanceLine(const spv_text text, spv_position position) { position->index++; return SPV_SUCCESS; default: - position->line++; + position->column++; position->index++; break; } @@ -124,7 +124,8 @@ spv_result_t spvTextAdvance(const spv_text text, spv_position position) { case '\0': return SPV_END_OF_STREAM; case ';': - return spvTextAdvanceLine(text, position); + if (spv_result_t error = spvTextAdvanceLine(text, position)) return error; + return spvTextAdvance(text, position); case ' ': case '\t': position->column++; diff --git a/test/Comment.cpp b/test/Comment.cpp new file mode 100644 index 0000000..c323023 --- /dev/null +++ b/test/Comment.cpp @@ -0,0 +1,47 @@ +// Copyright (c) 2015 The Khronos Group Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and/or associated documentation files (the +// "Materials"), to deal in the Materials without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Materials, and to +// permit persons to whom the Materials are 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 Materials. +// +// MODIFICATIONS TO THIS FILE MAY MEAN IT NO LONGER ACCURATELY REFLECTS +// KHRONOS STANDARDS. THE UNMODIFIED, NORMATIVE VERSIONS OF KHRONOS +// SPECIFICATIONS AND HEADER INFORMATION ARE LOCATED AT +// https://www.khronos.org/registry/ +// +// THE MATERIALS ARE 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 +// MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS. + +#include "TestFixture.h" +#include "UnitSPIRV.h" + +TEST_F(TextToBinaryTest, Whitespace) { + SetText(R"( +; I'm a proud comment at the begining of the file +; I hide: OpCapability Shader + OpMemoryModel Logical Simple ; comment after instruction +;;;;;;;; many ;'s + %glsl450 = OpExtInstImport "GLSL.std.450" + ; comment indented +)"); + EXPECT_EQ(SPV_SUCCESS, spvTextToBinary(&text, opcodeTable, operandTable, + extInstTable, &binary, &diagnostic)); + if (binary) { + spvBinaryDestroy(binary); + } + if (diagnostic) { + spvDiagnosticPrint(diagnostic); + } +} diff --git a/test/TextAdvance.cpp b/test/TextAdvance.cpp index 969a869..423ef9f 100644 --- a/test/TextAdvance.cpp +++ b/test/TextAdvance.cpp @@ -66,6 +66,23 @@ TEST(TextAdvance, LeadingNewLinesSpacesAndTabs) { ASSERT_EQ(5, position.index); } +TEST(TextAdvance, LeadingWhitespaceAfterCommentLine) { + char textStr[] = "; comment\n \t \tWord"; + spv_text_t text = {textStr, strlen(textStr)}; + spv_position_t position = {}; + ASSERT_EQ(SPV_SUCCESS, spvTextAdvance(&text, &position)); + ASSERT_EQ(4, position.column); + ASSERT_EQ(1, position.line); + ASSERT_EQ(14, position.index); +} + +TEST(TextAdvance, EOFAfterCommentLine) { + char textStr[] = "; comment"; + spv_text_t text = {textStr, strlen(textStr)}; + spv_position_t position = {}; + ASSERT_EQ(SPV_END_OF_STREAM, spvTextAdvance(&text, &position)); +} + TEST(TextAdvance, NullTerminator) { char textStr[] = ""; spv_text_t text = {textStr, strlen(textStr)}; diff --git a/test/TextToBinary.cpp b/test/TextToBinary.cpp index f68ca66..8176415 100644 --- a/test/TextToBinary.cpp +++ b/test/TextToBinary.cpp @@ -47,15 +47,15 @@ TEST(TextToBinary, Default) { OpSourceExtension "PlaceholderExtensionName" OpEntryPoint Kernel $1 OpExecutionMode $1 LocalSizeHint 1 1 1 -%2 = OpTypeVoid -%3 = OpTypeBool -; commment -%4 = OpTypeInt 8 0 ; comment -%5 = OpTypeInt 8 1 -%6 = OpTypeInt 16 0 -%7 = OpTypeInt 16 1 -%8 = OpTypeInt 32 0 -%9 = OpTypeInt 32 1 + %2 = OpTypeVoid + %3 = OpTypeBool + ; commment + %4 = OpTypeInt 8 0 ; comment + %5 = OpTypeInt 8 1 + %6 = OpTypeInt 16 0 + %7 = OpTypeInt 16 1 + %8 = OpTypeInt 32 0 + %9 = OpTypeInt 32 1 %10 = OpTypeInt 64 0 %11 = OpTypeInt 64 1 %12 = OpTypeFloat 16 -- 2.7.4