Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue# 168: Fix invalidHashHash exception on valid token. #169

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Issue# 168: Fix invalidHashHash exception on valid token. #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion simplecpp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1803,7 +1803,9 @@ namespace simplecpp {
throw invalidHashHash(tok->location, name());

bool canBeConcatenatedWithEqual = A->isOneOf("+-*/%&|^") || A->str() == "<<" || A->str() == ">>";
if (!A->name && !A->number && A->op != ',' && !A->str().empty() && !canBeConcatenatedWithEqual)
bool isStringConstant = A->str().length() >= 2U && *A->str().begin() == '"' && *A->str().rbegin() == '"';
bool isCharConstant = A->str().length() == 3U && *A->str().begin() == '\'' && *A->str().rbegin() == '\'';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err.. suppose ''' is a character constant not covered by the rule it's defining 😆

if (!A->name && !A->number && A->op != ',' && !A->str().empty() && !canBeConcatenatedWithEqual && !isStringConstant && !isCharConstant)
throw invalidHashHash(tok->location, name());

Token *B = tok->next->next;
Expand All @@ -1814,6 +1816,9 @@ namespace simplecpp {
(!canBeConcatenatedWithEqual && B->op == '='))
throw invalidHashHash(tok->location, name());

if ((isStringConstant || isCharConstant) && !B->name)
throw invalidHashHash(tok->location, name());

std::string strAB;

const bool varargs = variadic && args.size() >= 1U && B->str() == args[args.size()-1U];
Expand Down
26 changes: 26 additions & 0 deletions test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,31 @@ static void hashhash9()
ASSERT_EQUALS("file0,1,syntax_error,failed to expand 'A', Invalid ## usage when expanding 'A'.\n", toString(outputList));
}

static void hashhash10() //Paste string constant, integer, real to user defined literal
{
const char *code = "#define A(x) x##_y\n"
"A(\"A\");\n"
"A('A');\n"
"A(1.0);\n"
"A(1);";

const char expected[] = "\n\"A\"_y ;\n"
"'A'_y ;\n"
Copy link
Owner

@danmar danmar May 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure we should generate such tokens. If a token starts with a ", I believe it will be classified as a string by cppcheck.

How about outputting something like operator_y("A") .. 4 tokens?

There was some tweak in Cppcheck for these operators a while ago I am not sure how that works. Can you double check? It would be good if we match that. Define such operator and look at the debug output cppcheck --debug.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied these changes to simplecpp in cppcheck and ran cppcheck on the below file:

example.cpp

#include <cstdlib>

#define NUM_CHAR(str) str##_numch
#define X(str) str##_x
#define Y(ch) ch##_y
#define Z(val) val##_z

inline auto operator "" _x(const char *ch, std::size_t size) noexcept -> std::size_t
{
  return size;
}

inline auto operator "" _y(char ch) noexcept -> double
{
  return static_cast<double>(ch) * 0.25;
}

inline auto operator ""_z(long double val) noexcept -> const char *
{
  return (val < 0.5) ? "ABC" : "EFG";
}

int main()
{
  auto a = "abc"_x;
  auto b = X("abc");
  auto c = 't'_y;
  auto d = Y('t');
  auto e = 1.0_z;
  auto f = Z(1.0);

  auto g = operator"" _x( "abc", 3);
  auto h = operator"" _x("abc"); // Wrong signature
}

As you thought, the change caused an error to be generated by cppcheck. I removed the macros and ran it again to see what it produces normally, and made the following further change to try to mimic how it appeared to be handling them:

// @line 1842:
if (isStringConstant || isCharConstant)
  output->push_back(new Token(*B));
else if (varargs ... //etc

With this change, cppcheck runs and does not produce an error. Some select output from --debug and --verbose below:

1:
|
7:
8: auto operator""_x ( const char * ch@1 , unsigned long size@2 ) noexcept . unsigned long
9: {
10: return size@2 ;
11: }
12:
13: auto operator""_y ( char ch@3 ) noexcept . double
14: {
15: return static_cast < double > ( ch@3 ) * 0.25 ;
16: }
17:
18: auto operator""_z ( long double val@4 ) noexcept . const char *
19: {
20: return ( val@4 < 0.5 ) ? "ABC" : "EFG" ;
21: }
22:
23: int main ( )
24: {
25: auto a@5 ; a@5 = "abc" _x ;
26: auto b@6 ; b@6 = "abc" _x
5:
|
25:
26: ;
27: auto c@7 ; c@7 = 't' _y ;
28: auto d@8 ; d@8 = 't' _y
6:
|
27:
28: ;
29: auto e@9 ; e@9 = 1.0_z ;
30: auto f@10 ; f@10 = 1.0_z ;
31:
32: auto g@11 ; g@11 = operator""_x ( "abc" , 3 ) ;
33: auto h@12 ; h@12 = operator""_x ( "abc" ) ;
34: }

##AST
(
|-operator""_x
`-,
  |-*
  | |-char
  | `-ch 'const signed char *'
  `-size 'unsigned long'

return ''
`-size 'unsigned long'

(
|-operator""_y
`-ch 'signed char'

return ''
`-* 'double'
  |-( 'double'
  | |-static_cast
  | `-ch 'signed char'
  `-0.25 'double'

(
|-operator""_z
`-val 'long double'

*
|-.
| |-noexcept
| `-char
`-{

( 'signed int'
`-main

= 'const char *'
|-a 'const char *'
`-"abc" 'const char *'

= 'const char *'
|-b 'const char *'
`-"abc" 'const char *'

= 'char'
|-c 'char'
`-'t' 'char'

= 'char'
|-d 'char'
`-'t' 'char'

=
|-e
`-1.0_z

=
|-f
`-1.0_z

=
|-g
`-(
  |-operator""_x
  `-,
    |-"abc" 'const char *'
    `-3 'signed int'

=
|-h
`-(
  |-operator""_x
  `-"abc" 'const char *'

From that it looks like just splitting it into two tokens and discarding the ## is doing the same as cppcheck is right now. It seems cppcheck is mostly just ignoring these operators right now. It has at least got the type wrong for all cases.

Should we go ahead with this to make it like how cppcheck processes it, or try operator"" {TokenB} ( {TokenA} ) which might give more meaningful output in cppcheck (although it still doesn't seem to get the return type from the function).?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! That is a good investigation. I feel much more comfortable now. :-)

Should we go ahead with this to make it like how cppcheck processes it, or try operator"" {TokenB} ( {TokenA} ) which might give more meaningful output in cppcheck (although it still doesn't seem to get the return type from the function).?

I suggest the first option for now. We need to handle this better in cppcheck.

"1.0_y ;\n"
"1_y ;";

ASSERT_EQUALS(expected, preprocess(code));

const simplecpp::DUI dui;
simplecpp::OutputList outputList;

code = "#define A(x) x##123\n"
"A(\"A\");";
outputList.clear();
preprocess(code, dui, &outputList);
ASSERT_EQUALS("file0,1,syntax_error,failed to expand 'A', Invalid ## usage when expanding 'A'.\n", toString(outputList));
}

static void hashhash_invalid_1()
{
std::istringstream istr("#define f(a) (##x)\nf(1)");
Expand Down Expand Up @@ -1882,6 +1907,7 @@ int main(int argc, char **argv)
TEST_CASE(hashhash7); // # ## # (C standard; 6.10.3.3.p4)
TEST_CASE(hashhash8);
TEST_CASE(hashhash9);
TEST_CASE(hashhash10);
TEST_CASE(hashhash_invalid_1);
TEST_CASE(hashhash_invalid_2);

Expand Down