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.

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

Eliminate $ from identifiers #26

Closed
genbattle opened this issue Jan 6, 2016 · 8 comments
Closed

Eliminate $ from identifiers #26

genbattle opened this issue Jan 6, 2016 · 8 comments
Labels

Comments

@genbattle
Copy link

Hi there,

I'm currently using lest to test a project of mine, and noticed it was producing some warnings when compiling tests for my project using clang (3.6 and 3.7):

/home/travis/build/genbattle/dkm/src/test/lest.hpp:121:41: error: 
      '$' in identifier
      [-Werror,-Wdollar-in-identifier-extension]
  ...$section = 0, $count = 1; $secti...
                               ^
/home/travis/build/genbattle/dkm/src/test/lest.hpp:121:52: error: 
      '$' in identifier
      [-Werror,-Wdollar-in-identifier-extension]
  ...0, $count = 1; $section < $count...
                               ^
/home/travis/build/genbattle/dkm/src/test/lest.hpp:121:60: error: 
      '$' in identifier
      [-Werror,-Wdollar-in-identifier-extension]
  ...= 1; $section < $count; $count -...
                             ^
/home/travis/build/genbattle/dkm/src/test/lest.hpp:121:73: error: 
      '$' in identifier
      [-Werror,-Wdollar-in-identifier-extension]
  ...< $count; $count -= 0==$section++ )

Clang is not happy about dollar characters $ being used in identifier names. This seems reasonable given the discussion here. Using such characters outside of the normal alphanumeric character set is implementation defined in C++11, and so may or may not work. On clang with -Wall -Wextra it happens to generate a warning.

Use of $ in identifiers should be removed if it can be without seriously impacting the readability or clarity of the code.

@genbattle genbattle changed the title Eliminate $ from variable names Eliminate $ from identifiers Jan 6, 2016
@martinmoene
Copy link
Owner

Thanks Nick,

I've become aware of $ being non-standard after already using it. It helps if someone else starts to mention it :)

I might change the names to:

  • lest_env
  • lest_count
  • lest_section

The 'problem' is that the user may not use these names for her own purpose.

Any other ideas?

@genbattle
Copy link
Author

Namespacing/label clashes are a difficult problem to solve with macros. Prefixing the names with lest_ seems like a reasonable solution.

@martinmoene
Copy link
Owner

As an aside:

For some time lest does contain :

#ifdef __clang__
# pragma clang diagnostic ignored "-Wdollar-in-identifier-extension"
...

@martinmoene
Copy link
Owner

@genbattle Ah, you're using lest version 1.24.1, current is 1.24.5.

1.24.2: This release fixes and suppresses several warnings for clang and gcc and fixes the version number for VC14 and higher.

@genbattle
Copy link
Author

I tried the latest version and it does indeed resolve all of my issues! Sorry for the trouble.

@martinmoene
Copy link
Owner

You're welcome.

Nonetheless it may be a good idea to stop using the $character.

@martinmoene martinmoene reopened this Jan 7, 2016
@Kosta-Github
Copy link

what about lest_internal__ as a prefix, very unlikely that someone else would use that too...

@martinmoene
Copy link
Owner

Thanks @Kosta-Github,

I chose to 'encode' the $ as lest_ and internal as _, obtaining somewhat shorter and easier to read names, such as lest__section.

The lest environment parameter named $ becomes lest_env as this may be of use to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants