erichkeane added a comment.

In D131307#3709595 <https://reviews.llvm.org/D131307#3709595>, @zatrazz wrote:

> This commit seems to have broken test-suite on aarch64 [1]. The warning shows:
>
>   FAILED: MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o
>   /home/adhemerval.zanella/projects/llvm/test-suite-build/tools/timeit 
> --summary MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.time 
> /home/adhemerval.zanella/projects/llvm/llvm-src-build-stage1-buildbot/bin/clang++
>  -DNDEBUG  -O3 -DNDEBUG   -w -Werror=date-time -DNOASM -DLLVM -MD -MT 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -MF 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.d -o 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -c 
> /home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp
>   
> /home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp:3606:24:
>  error: integer value -1 is outside the valid range of values [0, 15] for 
> this enumeration type [-Wenum-constexpr-conversion]
>       if (c==EOF) return (Filetype)(-1);
>                          ^
>   1 error generated.
>
> I am not sure if this is an existing issue with test-suite.
>
> [1] https://lab.llvm.org/staging/#/builders/158/builds/344

That IS Undefined Behavior, but I think was unintended by this patch.  The 
intent of this patch (and its parent) was to diagnose this UB during 
`constexpr` evaluation.  HOWEVER, this patch seems to have extended it to 
non-`constexpr` constant evaluation.  So while that _IS_ undefined behavior, I 
don't think it should be covered by this error (likely a normal 'warning' is 
fine here, but not this diagnostic).  Hopefully @shafik can correct this when 
he starts again today.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131307/new/

https://reviews.llvm.org/D131307

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

Reply via email to