Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Add Clang warning for register qulifiers #21984

Open Quuxplusone opened 9 years ago

Quuxplusone commented 9 years ago
Bugzilla Link PR21985
Status REOPENED
Importance P enhancement
Reported by Eugene Zelenko (eugene.zelenko@gmail.com)
Reported on 2014-12-19 12:08:15 -0800
Last modified on 2015-04-10 15:18:42 -0700
Version unspecified
Hardware All All
CC hans@chromium.org, llvm-bugs@lists.llvm.org, rnk@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Hi!

I think will be good idea to introduce warning for usage of register qualifier: most likely useless for optimization, but may still present in legacy code (when number of register variables may be much more then number of actual registers).

Eugene.

Quuxplusone commented 9 years ago
It's under -Wdeprecated-register, and it's on by default:

$ cat t.cpp
void use(int);
int main() {
  register int a = 42;
  use(a);
}

$ clang -c t.cpp
t.cpp:3:3: warning: 'register' storage class specifier is deprecated [-
Wdeprecated-register]
  register int a = 42;
  ^~~~~~~~~
1 warning generated.
Quuxplusone commented 9 years ago
(In reply to comment #1)
> It's under -Wdeprecated-register, and it's on by default:
>
> $ cat t.cpp
> void use(int);
> int main() {
>   register int a = 42;
>   use(a);
> }
>
> $ clang -c t.cpp
> t.cpp:3:3: warning: 'register' storage class specifier is deprecated
> [-Wdeprecated-register]
>   register int a = 42;
>   ^~~~~~~~~
> 1 warning generated.

Hi, Reid!

I tried 3.5 on Linux and r223387 on Windows but it didn't produce warning for C
code.

According to
http://llvm.org/klaus/clang/commit/58df0426944c1bf8a80b6935bbea3815fc506474/ it
enabled for C++11.

Is it possible to enable it for C and C++ of older standards? Even not by
default?

Eugene.
Quuxplusone commented 9 years ago

'register' is not deprecated in C, so -Wdeprecated-register doesn't seem like the right place for it.

I can see the desire to avoid using 'register' in new code, but it's also quite harmless so it seems more like a style issue than something to warn about.

Quuxplusone commented 9 years ago
(In reply to comment #3)
> 'register' is not deprecated in C, so -Wdeprecated-register doesn't seem
> like the right place for it.
>
> I can see the desire to avoid using 'register' in new code, but it's also
> quite harmless so it seems more like a style issue than something to warn
> about.

May be this warning could be implemented in Clang-Tidy, if it task is to fix
style issues?
Quuxplusone commented 9 years ago
(In reply to comment #3)
> 'register' is not deprecated in C, so -Wdeprecated-register doesn't seem
> like the right place for it.
>
> I can see the desire to avoid using 'register' in new code, but it's also
> quite harmless so it seems more like a style issue than something to warn
> about.

True, but I think if the user explicitly passes -Wdeprecated-register on the
command line, then they should get the warning that we already have code for.
Normally, we use DefaultIgnore for this, but for this language-specific mode we
would need to add extra logic. I think we could take a patch for that.

This warning should still be off by default in C. C code has a habit of staying
old and crufty, while C++ seems to update or die.
Quuxplusone commented 9 years ago

Parasoft C/C++test has this check (JSF-140-2).