[PATCH] D32525: [clang-format] Add SpaceBeforeColon option
karies added a comment. Does this qualify? https://github.com/root-project/root/blob/master/.clang-format#L84 # You want this : enable it if you have https://reviews.llvm.org/D32525 # SpaceBeforeColon: false in ROOT's `.clang-format`. https://reviews.llvm.org/D32525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34439: Add GCC's noexcept-type alias for c++1z-compat-mangling
karies added a comment. For the record, here's what GCC does (from https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html#C_002b_002b-Dialect-Options which probably has a typo, the second -Wnoexcept is likely meant to be -Wnoexcept-type): "Enabled by -Wabi and -Wc++1z-compat." -Wc++1z-compat is probably meant to enable a group out of which -Wnoexcept-type is just one (currently the only one as far as I can see). FWIW, we notice the missing -Wno-noexcept-type but not -Wc++1z-compat. https://reviews.llvm.org/D34439 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D65956: clang: Diag running out of file handles while looking for files
karies added a comment.
Herald added a project: All.
Similar to the concern raised at
https://github.com/llvm/llvm-project/commit/50fcf7285eeb001d751eadac5d001a0e216fd701
we have received user reports about this patch: with `-Ino-access-permissions
-Iall-good`, clang will throw an error (EACCES) even though header search goes
on and will find the header in `all-good`. That seems a misleading an
unnecessary error, especially as the header *is* found later, yet compilation
"fails" because of this diagnostic.
I would propose to collect these errors, and where originally (before this
patch), cling would complain "file not found" we could diagnose "while
searching for this header, the following errors have been seen" and reporting
the uniq-ed set of collected diags - but *only* in the case where the file
cannot be found. This would prevent spurious diags during iteration through the
SearchDirs when HeaderSearch actually finds the file. And I am fully aware that
it's pointless to propose something without providing a differential :-/
FYI here's what we do right now to handle the EACCES case:
patch
--- a/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
+++ b/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
@@ -367,7 +367,9 @@ Optional
HeaderSearch::getFileAndSuggestModule(
std::error_code EC = llvm::errorToErrorCode(File.takeError());
if (EC != llvm::errc::no_such_file_or_directory &&
EC != llvm::errc::invalid_argument &&
-EC != llvm::errc::is_a_directory && EC !=
llvm::errc::not_a_directory) {
+EC != llvm::errc::is_a_directory &&
+EC != llvm::errc::not_a_directory &&
+EC != llvm::errc::permission_denied) {
Diags.Report(IncludeLoc, diag::err_cannot_open_file)
<< FileName << EC.message();
}
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65956/new/
https://reviews.llvm.org/D65956
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34912: Handle cases where the value is too large to fit into the underlying type.
karies added a comment.
Two comments:
- I think the part `Val.getBitWidth() == 64` likely needs rewording, although I
don't know how
- I can show the motivation, not sure whether that qualifies as a test case:
$ echo 'template struct S{}; S<(unsigned long long)-1> s
= 42;' | clang++ -fsyntax-only -std=c++14 -x c++ -
:1:69: error: no viable conversion from 'int' to 'S<(unsigned long
long)-1>'
template struct S{}; S<(unsigned long long)-1> s = 42;
^ ~~
:1:38: note: candidate constructor (the implicit copy constructor) not
viable: no known conversion from 'int' to 'const S<18446744073709551615> &' for
1st argument
template struct S{}; S<(unsigned long long)-1> s = 42;
^ ~~
:1:38: note: candidate constructor (the implicit move constructor) not
viable: no known conversion from 'int' to 'S<18446744073709551615> &&' for 1st
argument
template struct S{}; S<(unsigned long long)-1> s = 42;
^ ~~
1 error generated.
The following shows that the name clang usew for diagnostics
(`S<18446744073709551615> &`) is actually invalid:
$ echo 'template struct S{}; S<18446744073709551615> a;'
| clang++ -fsyntax-only -std=c++14 -x c++ -
:1:45: warning: integer literal is too large to be represented in a
signed integer type, interpreting as unsigned [-Wimplicitly-unsigned-literal]
template struct S{}; S<18446744073709551615> a;
^
1 warning generated.
My goal is for clang to print a valid class name, which means this should be
spelled S<18446744073709551615ull>`.
Repository:
rL LLVM
https://reviews.llvm.org/D34912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34985: Do not read the file to determine its name.
karies added a comment. To be clear: emitting annotations will trigger the determination of `PresumedLoc`s. As part of that (but not the first part, IIRC) `SourceManager::getBufferName(()` will be called which will trigger the `fopen` of the file, just to get its name. Another task of `PresumedLoc` is to determine the line number which triggers an `fopen`; @v.g.vassilev is looking into a possible solution for that one. Repository: rL LLVM https://reviews.llvm.org/D34985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34985: Do not read the file to determine its name.
karies added a comment. In https://reviews.llvm.org/D34985#801498, @bruno wrote: > @v.g.vassilev will this also benefit from `ContentCache::getBuffer` > improvements from https://reviews.llvm.org/D33275? I guess if `getBuffer` > transparently handles pointing to right buffer we won't need this change? This change is for cases where you have no buffer, because the file was never `fopen()`ed, because it's an `ASTReader` `InputFile`. Repository: rL LLVM https://reviews.llvm.org/D34985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30000: Fix for pr31836 - pp_nonportable_path on absolute paths: broken delimiters
karies added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1983 + isLeadingSeparator = false; +else + Path.append(Component); eric_niebler wrote: > What happens on Windows for an absolute path like "C:/hello/world.h", I > wonder? Does this correctly generate the fixit? @karies? I also cannot reproduce this diag on Windows, likely because NTFS and FAT are case indifferent (not preserving like MacOS) so the file system cannot reply with "this is how *I* would have spelled the file name". https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30000: Fix for pr31836 - pp_nonportable_path on absolute paths: broken delimiters
karies added inline comments. Comment at: test/Lexer/case-insensitive-include-pr31836.sh:6 +// RUN: touch %T/case-insensitive-include-pr31836.h +// RUN: echo "#include \"%T/Case-Insensitive-Include-Pr31836.h\"" | %clang_cc1 -E - 2>&1 | FileCheck %s + @twoh Does that actually work on Linux? I thought (most?) Linux file systems are case sensitive? I.e. I'd expect to get a "file not found" diag on the `#include`. https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.
karies added inline comments.
Comment at: clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt:2
+# The interpreter can throw an exception from user input. The test binary needs
+# to be compiled with exception support to expect and catch the thrown
+# exception.
I don't understand the term "to expect" the thrown exception. Please ignore if
that's a known term of art.
Comment at:
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:93
+static void ThrowerAnError(const char* Name) {
+ throw std::runtime_error(Name);
+}
Why not just `throw Name;` to avoid `#include `?
Comment at:
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:100
+ } catch (const std::exception& E) {
+printf("Caught: '%s'\n", E.what());
+ } catch (...) {
Consider fwd declaring `printf` to avoid inclusion of `stdio.h`.
Comment at:
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:102
+ } catch (...) {
+printf("Unknown exception\n");
+ }
How is that provoking a test failure? What about `exit(1)` or whatever works
for gtest?
Comment at:
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:104
+ }
+ ThrowerAnError("From JIT");
+ return 0;
To me, the wording difference between "In JIT" and "From JIT" doesn't signal
anything. Maybe "the first could be "To be caught in JIT" and the second "To be
caught in binary"?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107049/new/
https://reviews.llvm.org/D107049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
