0x8000-0000 marked an inline comment as done.
0x8000-0000 added inline comments.


================
Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63
+configuration for accepted floating point values, primarily because most
+floating point comparisons are not exact, and some of the exact ones are not
+portable.
----------------
lebedev.ri wrote:
> aaron.ballman wrote:
> > 0x8000-0000 wrote:
> > > aaron.ballman wrote:
> > > > I am curious to know how true this is. You got some data for integer 
> > > > values and reported it, but I'm wondering if you've tried the same 
> > > > experiment with floating-point numbers?
> > > The problem with the floating point numbers as text is: they need to be 
> > > parsed both from the configuration and from the source code _then_ 
> > > compared. What is an acceptable epsilon? I don't know. Is the same 
> > > epsilon acceptable on all source code? I don't know.
> > Yeah, I'm not too worried about the situations in which the epsilon 
> > matters. I'm more worried that we'll see a lot of 1.0, 2.0 floating-point 
> > literals where the floating-point value is a nice, round, easy-to-represent 
> > number but users have no way to disable this diagnostic short of `const 
> > float Two = 2.0f;`
> Random thought: the types that are ignored should/could be configurable, i.e. 
> there should be a switch
> whether or not to complain about floats.
Even though they might be nice and round... they should mean _something_ other 
than 'Two'.

The thing is... magic integers are used as buffer sizes, or to map things that 
are discrete in nature - number of legs of a typical mammal for instance. Not 
sure what magic numbers exist in nature besides pi and e and some fundamental 
physical constants )Avogadro's number, etc). But even there, it is better to 
use a symbolic constant.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to