[PATCH] D90587: [clangd] Control the delay between index hot reloading in remote-server-index

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:78
+llvm::cl::desc("Delay between index hot reload checks (in seconds)"),
+llvm::cl::init(90),
+llvm::cl::Hidden,

this was 30 before. not that it matters.



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:79
+llvm::cl::init(90),
+llvm::cl::Hidden,
+};

i think this might be useful in general, as servers will have different pull 
frequency for newer index. so maybe make it non-hidden.



Comment at: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py:35
   'clangd-index-server', '--server-address=' + server_address,
-  args.index_file, args.project_root
+  '--hot-reload-frequency=3', args.index_file, args.project_root
   ],

let's set it to 1. clangd tests already takes about 3-4 seconds to run on (our) 
workstations. it feels like this might end up increasing the test latency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90587

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

[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Thanks for figuring this one out!

I think the final result looks pretty good, I especially loved only disabling 
when there's a request in question.
My only hesitation is around logging all severities when we are outside a 
request, I suppose it shouldn't cause too much trouble but we sometimes end up 
logging too much in verbose mode and I am afraid of this hindering the server 
performance at random intervals (e.g. when it loads a new index). But this is 
just speculation. I suppose we can think about this when it happens. Also maybe 
we should not be logging that much instead :D

In addition to that it could be nice to see how the `public` tagging will 
happen. Are you planning to have a custom endpoint for it, or should the user 
manually insert it into format string? Both are fine by me, the latter might 
even make it less error-prone for accidental logging at the cost of misspells 
and spreading the string across the codebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90526

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

[PATCH] D90590: [clangd] Improve remote-index test

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
kbobyrev requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90590

Files:
  clang-tools-extra/clangd/test/remote-index/pipeline_helper.py


Index: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
===
--- clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
+++ clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
@@ -14,7 +14,13 @@
 from socket import socket
 import sys
 import time
-import signal
+import threading
+
+
+def kill_server_after_delay(server_process):
+  time.sleep(10)
+  if server_process.poll() is None:
+server_process.kill()
 
 
 def main():
@@ -36,11 +42,17 @@
   ],
   stderr=subprocess.PIPE)
 
+  # This will kill index_server_process if it hangs without printing init
+  # message.
+  shutdown_thread = threading.Thread(
+  target=kill_server_after_delay, args=(index_server_process,))
+  shutdown_thread.daemon = True
+  shutdown_thread.start()
+
   # Wait for the server to warm-up.
-  time.sleep(4)
   found_init_message = False
-  for line in index_server_process.stderr:
-if b'Server listening' in line:
+  while index_server_process.poll() is None:
+if b'Server listening' in index_server_process.stderr.readline():
   found_init_message = True
   break
 
@@ -56,7 +68,7 @@
 stdin=in_file)
 
   clangd_process.wait()
-  os.kill(index_server_process.pid, signal.SIGINT)
+  index_server_process.kill()
 
 
 if __name__ == '__main__':


Index: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
===
--- clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
+++ clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
@@ -14,7 +14,13 @@
 from socket import socket
 import sys
 import time
-import signal
+import threading
+
+
+def kill_server_after_delay(server_process):
+  time.sleep(10)
+  if server_process.poll() is None:
+server_process.kill()
 
 
 def main():
@@ -36,11 +42,17 @@
   ],
   stderr=subprocess.PIPE)
 
+  # This will kill index_server_process if it hangs without printing init
+  # message.
+  shutdown_thread = threading.Thread(
+  target=kill_server_after_delay, args=(index_server_process,))
+  shutdown_thread.daemon = True
+  shutdown_thread.start()
+
   # Wait for the server to warm-up.
-  time.sleep(4)
   found_init_message = False
-  for line in index_server_process.stderr:
-if b'Server listening' in line:
+  while index_server_process.poll() is None:
+if b'Server listening' in index_server_process.stderr.readline():
   found_init_message = True
   break
 
@@ -56,7 +68,7 @@
 stdin=in_file)
 
   clangd_process.wait()
-  os.kill(index_server_process.pid, signal.SIGINT)
+  index_server_process.kill()
 
 
 if __name__ == '__main__':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90587: [clangd] Control the delay between index hot reloading in remote-server-index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev abandoned this revision.
kbobyrev added a comment.

As discussed in PM, it probably doesn't make sense right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90587

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

[PATCH] D90376: [clangd] Add requests logging to clangd-index-server

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev abandoned this revision.
kbobyrev added a comment.

In favor of D90526 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90376

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

[PATCH] D90376: [clangd] Add requests logging to clangd-index-server

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev reclaimed this revision.
kbobyrev added a comment.

Actually, maybe I should just postone and build on top of that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90376

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

[PATCH] D87831: [clang] Expose helper function to turn PP keywords spelling into PPKeywordKind

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev planned changes to this revision.
kbobyrev added a comment.
Herald added a subscriber: dexonsmith.

Not needed yet, waiting for myself to start adding some PP-based FoldingRanges 
to continue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87831

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

[PATCH] D90570: [mips] Add a -mmips3d command line option to clang

[Simon Atanasyan via Phabricator via cfe-commits]

atanasyan added a comment.

What's a goal of this change? Do you want to suppress an error message when the 
option provided to Clang? If so, is it a real-life case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90570

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

[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D90526#2367659 , @kadircet wrote:

> Thanks for figuring this one out!
>
> I think the final result looks pretty good, I especially loved only disabling 
> when there's a request in question.
> My only hesitation is around logging all severities when we are outside a 
> request, I suppose it shouldn't cause too much trouble but we sometimes end 
> up logging too much in verbose mode and I am afraid of this hindering the 
> server performance at random intervals (e.g. when it loads a new index). But 
> this is just speculation. I suppose we can think about this when it happens. 
> Also maybe we should not be logging that much instead :D

So I can't remember anymore why I put this as an enum `-log=public` rather than 
an independent switch `-log-public` or so that could combine with any log level.
Sounds like the latter would be better?

> In addition to that it could be nice to see how the `public` tagging will 
> happen. Are you planning to have a custom endpoint for it, or should the user 
> manually insert it into format string? Both are fine by me, the latter might 
> even make it less error-prone for accidental logging at the cost of misspells 
> and spreading the string across the codebase.

Manually insert it into the format string. After thinking about it I came to 
the conclusion there just aren't that many places where we need to log things 
publicly. I expect to basically not have any visibility inside most requests 
except the error patterns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90526

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

[PATCH] D90528: [clangd] Fix check-clangd with no clang built

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90528

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

[PATCH] D60193: [OpenCL] Added addrspace_cast operator

[Hans Wennborg via Phabricator via cfe-commits]

hans added a subscriber: tstellar.
hans added inline comments.



Comment at: clang/include/clang-c/Index.h:2057
+   */
+  CXCursor_CXXAddrspaceCastExpr = 129,
+

Anastasia wrote:
> hans wrote:
> > akyrtzi wrote:
> > > Hi Anastasia, apologies for not catching this earlier, but libclang is 
> > > intended to keep a stable ABI and changing the enumerator values breaks 
> > > libclang's ABI compatibility.
> > > 
> > > Would you be able to make a follow-up commit and restore the enumerator 
> > > values to their original values? I would suggest to add 
> > > `CXCursor_CXXAddrspaceCastExpr` at the end and assign to it the next 
> > > available value that is not already taken.
> > It also broke the Python bindings, as someone reported here: 
> > https://stackoverflow.com/questions/64542827/there-appears-to-be-mismatch-in-clang-llvm-ver-11-0-0-python-bindings
> I guess it's too late to fix 11.0.0, does it make sense to fix 11.0.1?
That's @tstellar 's call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60193

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

[PATCH] D90590: [clangd] Improve remote-index test

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd0beda1b6661: [clangd] Improve remote-index test (authored 
by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90590

Files:
  clang-tools-extra/clangd/test/remote-index/pipeline_helper.py


Index: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
===
--- clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
+++ clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
@@ -14,7 +14,13 @@
 from socket import socket
 import sys
 import time
-import signal
+import threading
+
+
+def kill_server_after_delay(server_process):
+  time.sleep(10)
+  if server_process.poll() is None:
+server_process.kill()
 
 
 def main():
@@ -36,11 +42,17 @@
   ],
   stderr=subprocess.PIPE)
 
+  # This will kill index_server_process if it hangs without printing init
+  # message.
+  shutdown_thread = threading.Thread(
+  target=kill_server_after_delay, args=(index_server_process,))
+  shutdown_thread.daemon = True
+  shutdown_thread.start()
+
   # Wait for the server to warm-up.
-  time.sleep(4)
   found_init_message = False
-  for line in index_server_process.stderr:
-if b'Server listening' in line:
+  while index_server_process.poll() is None:
+if b'Server listening' in index_server_process.stderr.readline():
   found_init_message = True
   break
 
@@ -56,7 +68,7 @@
 stdin=in_file)
 
   clangd_process.wait()
-  os.kill(index_server_process.pid, signal.SIGINT)
+  index_server_process.kill()
 
 
 if __name__ == '__main__':


Index: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
===
--- clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
+++ clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
@@ -14,7 +14,13 @@
 from socket import socket
 import sys
 import time
-import signal
+import threading
+
+
+def kill_server_after_delay(server_process):
+  time.sleep(10)
+  if server_process.poll() is None:
+server_process.kill()
 
 
 def main():
@@ -36,11 +42,17 @@
   ],
   stderr=subprocess.PIPE)
 
+  # This will kill index_server_process if it hangs without printing init
+  # message.
+  shutdown_thread = threading.Thread(
+  target=kill_server_after_delay, args=(index_server_process,))
+  shutdown_thread.daemon = True
+  shutdown_thread.start()
+
   # Wait for the server to warm-up.
-  time.sleep(4)
   found_init_message = False
-  for line in index_server_process.stderr:
-if b'Server listening' in line:
+  while index_server_process.poll() is None:
+if b'Server listening' in index_server_process.stderr.readline():
   found_init_message = True
   break
 
@@ -56,7 +68,7 @@
 stdin=in_file)
 
   clangd_process.wait()
-  os.kill(index_server_process.pid, signal.SIGINT)
+  index_server_process.kill()
 
 
 if __name__ == '__main__':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] d0beda1 - [clangd] Improve remote-index test

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2020-11-02T10:55:17+01:00
New Revision: d0beda1b66617cda8a1f54978fca72832496b1fb

URL: 
https://github.com/llvm/llvm-project/commit/d0beda1b66617cda8a1f54978fca72832496b1fb
DIFF: 
https://github.com/llvm/llvm-project/commit/d0beda1b66617cda8a1f54978fca72832496b1fb.diff

LOG: [clangd] Improve remote-index test

Introduce a separate thread that will kill `clangd-index-server` after 10 
seconds regardless. This helps shut down the test if the server hangs and 
`stderr.readline()` does not contain inititalizatiton message. It prevents 
"necessary" waiting delay for the server warm-up and only introduces additional 
delay if the test fails.

It also makes use of `subprocess.Popen.kill()` which is a portable way of 
handling process shutdown and avoids using signals.

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D90590

Added: 


Modified: 
clang-tools-extra/clangd/test/remote-index/pipeline_helper.py

Removed: 




diff  --git a/clang-tools-extra/clangd/test/remote-index/pipeline_helper.py 
b/clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
index 99380c445be6..d062e29bd8ee 100644
--- a/clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
+++ b/clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
@@ -14,7 +14,13 @@
 from socket import socket
 import sys
 import time
-import signal
+import threading
+
+
+def kill_server_after_delay(server_process):
+  time.sleep(10)
+  if server_process.poll() is None:
+server_process.kill()
 
 
 def main():
@@ -36,11 +42,17 @@ def main():
   ],
   stderr=subprocess.PIPE)
 
+  # This will kill index_server_process if it hangs without printing init
+  # message.
+  shutdown_thread = threading.Thread(
+  target=kill_server_after_delay, args=(index_server_process,))
+  shutdown_thread.daemon = True
+  shutdown_thread.start()
+
   # Wait for the server to warm-up.
-  time.sleep(4)
   found_init_message = False
-  for line in index_server_process.stderr:
-if b'Server listening' in line:
+  while index_server_process.poll() is None:
+if b'Server listening' in index_server_process.stderr.readline():
   found_init_message = True
   break
 
@@ -56,7 +68,7 @@ def main():
 stdin=in_file)
 
   clangd_process.wait()
-  os.kill(index_server_process.pid, signal.SIGINT)
+  index_server_process.kill()
 
 
 if __name__ == '__main__':



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

[PATCH] D90397: [clangd] Value initialize SymbolIDs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 302230.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Mention possibility of returning null SymbolIDs in comments.
- Mark bool conversion operator explicit, fix the bug in returned value. (and 
run tests :))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90397

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CollectMacros.cpp
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolID.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2077,7 +2077,7 @@
   TestTU TU = TestTU::withCode(T.code());
   auto AST = TU.build();
   Symbol IndexSym;
-  IndexSym.ID = *getSymbolID(&findDecl(AST, "X"));
+  IndexSym.ID = getSymbolID(&findDecl(AST, "X"));
   IndexSym.Documentation = "comment from index";
   SymbolSlab::Builder Symbols;
   Symbols.insert(IndexSym);
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===
--- clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -96,7 +96,7 @@
   auto SID = getSymbolID(Macro->Name, Macro->Info, SM);
 
   EXPECT_THAT(ExpectedRefs,
-  UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[*SID]))
+  UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[SID]))
   << "Annotation=" << I << ", MacroName=" << Macro->Name
   << ", Test = " << Test;
 }
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -63,7 +63,7 @@
   // tradeoff. We expect the number of symbol references in the current file
   // is smaller than the limit.
   Req.Limit = 100;
-  Req.IDs.insert(*getSymbolID(&D));
+  Req.IDs.insert(getSymbolID(&D));
   llvm::Optional OtherFile;
   Index.refs(Req, [&](const Ref &R) {
 if (OtherFile)
@@ -244,7 +244,7 @@
 return;
   for (const auto *Target : Ref.Targets) {
 auto ID = getSymbolID(Target);
-if (!ID || TargetIDs.find(*ID) == TargetIDs.end())
+if (!ID || TargetIDs.find(ID) == TargetIDs.end())
   return;
   }
   Results.push_back(Ref.NameLoc);
@@ -304,7 +304,7 @@
size_t MaxLimitFiles) {
   trace::Span Tracer("FindOccurrencesOutsideFile");
   RefsRequest RQuest;
-  RQuest.IDs.insert(*getSymbolID(&RenameDecl));
+  RQuest.IDs.insert(getSymbolID(&RenameDecl));
 
   // Absolute file path => rename occurrences in that file.
   llvm::StringMap> AffectedFiles;
Index: clang-tools-extra/clangd/index/SymbolID.h
===
--- clang-tools-extra/clangd/index/SymbolID.h
+++ clang-tools-extra/clangd/index/SymbolID.h
@@ -15,6 +15,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -53,8 +54,11 @@
   std::string str() const;
   static llvm::Expected fromStr(llvm::StringRef);
 
+  bool isNull() const { return *this == SymbolID(); }
+  explicit operator bool() const { return !isNull(); }
+
 private:
-  std::array HashValue;
+  std::array HashValue{};
 };
 
 llvm::hash_code hash_value(const SymbolID &ID);
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -324,7 +324,7 @@
   // refs, because the indexing code only populates relations for specific
   // occurrences. For example, RelationBaseOf is only populated for the
   // occurrence inside the base-specifier.
-  processRelations(*ND, *ID, Relations);
+  processRelations(*ND, ID, Relations);
 
   bool CollectRef = static_cast(Opts.RefFilter & toRefKind(Roles));
   bool IsOnlyRef =
@@ -356,15 +356,15 @@
   if (!OriginalDecl)
 return true;
 
-  const Symbol *BasicSymbol = Symbols.find(*ID);
+  const Symbol *BasicSymbol = Symbols.find(ID);
   if (isPreferredDeclaration(*OriginalDecl, Roles))
 // If OriginalDe

[PATCH] D90397: [clangd] Value initialize SymbolIDs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/SymbolID.h:58
+  bool isNull() const { return HashValue != std::array{}; }
+  operator bool() const { return isNull(); }
+

sammccall wrote:
> sammccall wrote:
> > nit: I think you want this to be explicit. Note that if(x) **will** perform 
> > an explicit cast if needed
> this should be `!isNull()`!
oopsy, should've ran tests :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90397

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

[PATCH] D90397: [clangd] Value initialize SymbolIDs

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0df197516b69: [clangd] Value initialize SymbolIDs (authored 
by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90397

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CollectMacros.cpp
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolID.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2077,7 +2077,7 @@
   TestTU TU = TestTU::withCode(T.code());
   auto AST = TU.build();
   Symbol IndexSym;
-  IndexSym.ID = *getSymbolID(&findDecl(AST, "X"));
+  IndexSym.ID = getSymbolID(&findDecl(AST, "X"));
   IndexSym.Documentation = "comment from index";
   SymbolSlab::Builder Symbols;
   Symbols.insert(IndexSym);
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===
--- clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
@@ -96,7 +96,7 @@
   auto SID = getSymbolID(Macro->Name, Macro->Info, SM);
 
   EXPECT_THAT(ExpectedRefs,
-  UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[*SID]))
+  UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[SID]))
   << "Annotation=" << I << ", MacroName=" << Macro->Name
   << ", Test = " << Test;
 }
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -63,7 +63,7 @@
   // tradeoff. We expect the number of symbol references in the current file
   // is smaller than the limit.
   Req.Limit = 100;
-  Req.IDs.insert(*getSymbolID(&D));
+  Req.IDs.insert(getSymbolID(&D));
   llvm::Optional OtherFile;
   Index.refs(Req, [&](const Ref &R) {
 if (OtherFile)
@@ -244,7 +244,7 @@
 return;
   for (const auto *Target : Ref.Targets) {
 auto ID = getSymbolID(Target);
-if (!ID || TargetIDs.find(*ID) == TargetIDs.end())
+if (!ID || TargetIDs.find(ID) == TargetIDs.end())
   return;
   }
   Results.push_back(Ref.NameLoc);
@@ -304,7 +304,7 @@
size_t MaxLimitFiles) {
   trace::Span Tracer("FindOccurrencesOutsideFile");
   RefsRequest RQuest;
-  RQuest.IDs.insert(*getSymbolID(&RenameDecl));
+  RQuest.IDs.insert(getSymbolID(&RenameDecl));
 
   // Absolute file path => rename occurrences in that file.
   llvm::StringMap> AffectedFiles;
Index: clang-tools-extra/clangd/index/SymbolID.h
===
--- clang-tools-extra/clangd/index/SymbolID.h
+++ clang-tools-extra/clangd/index/SymbolID.h
@@ -15,6 +15,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -53,8 +54,11 @@
   std::string str() const;
   static llvm::Expected fromStr(llvm::StringRef);
 
+  bool isNull() const { return *this == SymbolID(); }
+  explicit operator bool() const { return !isNull(); }
+
 private:
-  std::array HashValue;
+  std::array HashValue{};
 };
 
 llvm::hash_code hash_value(const SymbolID &ID);
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -324,7 +324,7 @@
   // refs, because the indexing code only populates relations for specific
   // occurrences. For example, RelationBaseOf is only populated for the
   // occurrence inside the base-specifier.
-  processRelations(*ND, *ID, Relations);
+  processRelations(*ND, ID, Relations);
 
   bool CollectRef = static_cast(Opts.RefFilter & toRefKind(Roles));
   bool IsOnlyRef =
@@ -356,15 +356,15 @@
   if (!OriginalDecl)
 return true;
 
-  const Symbol *BasicSymbol = Symbols.find(*ID);
+  const Symbol *BasicSymbol = Symbols.find(ID);
   if (isPreferredDeclaration(*OriginalDecl, Roles))
 // If OriginalDecl is preferred, replace/create the existing canon

[clang-tools-extra] 0df1975 - [clangd] Value initialize SymbolIDs

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-02T11:37:47+01:00
New Revision: 0df197516b69a4477a4c8f02b7c4dccacda5f23f

URL: 
https://github.com/llvm/llvm-project/commit/0df197516b69a4477a4c8f02b7c4dccacda5f23f
DIFF: 
https://github.com/llvm/llvm-project/commit/0df197516b69a4477a4c8f02b7c4dccacda5f23f.diff

LOG: [clangd] Value initialize SymbolIDs

We were default initializing SymbolIDs before, which would leave
indeterminate values in underlying std::array.

This patch updates the underlying data initalization to be value-init and adds a
way to check for validness of a SymbolID.

Differential Revision: https://reviews.llvm.org/D90397

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CollectMacros.cpp
clang-tools-extra/clangd/HeaderSourceSwitch.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolID.h
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index ce1254f6693c..d1702b2d25b6 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -282,21 +282,20 @@ std::string printNamespaceScope(const DeclContext &DC) {
   return "";
 }
 
-llvm::Optional getSymbolID(const Decl *D) {
+SymbolID getSymbolID(const Decl *D) {
   llvm::SmallString<128> USR;
   if (index::generateUSRForDecl(D, USR))
-return None;
+return {};
   return SymbolID(USR);
 }
 
-llvm::Optional getSymbolID(const llvm::StringRef MacroName,
- const MacroInfo *MI,
- const SourceManager &SM) {
+SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
+ const SourceManager &SM) {
   if (MI == nullptr)
-return None;
+return {};
   llvm::SmallString<128> USR;
   if (index::generateUSRForMacro(MacroName, MI->getDefinitionLoc(), SM, USR))
-return None;
+return {};
   return SymbolID(USR);
 }
 

diff  --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index c8b6008339e2..1e3447376c10 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -64,19 +64,18 @@ std::string printName(const ASTContext &Ctx, const 
NamedDecl &ND);
 /// string if decl is not a template specialization.
 std::string printTemplateSpecializationArgs(const NamedDecl &ND);
 
-/// Gets the symbol ID for a declaration, if possible.
-llvm::Optional getSymbolID(const Decl *D);
+/// Gets the symbol ID for a declaration. Returned SymbolID might be null.
+SymbolID getSymbolID(const Decl *D);
 
-/// Gets the symbol ID for a macro, if possible.
+/// Gets the symbol ID for a macro. Returned SymbolID might be null.
 /// Currently, this is an encoded USR of the macro, which incorporates macro
 /// locations (e.g. file name, offset in file).
 /// FIXME: the USR semantics might not be stable enough as the ID for index
 /// macro (e.g. a change in definition offset can result in a 
diff erent USR). We
 /// could change these semantics in the future by reimplementing this funcure
 /// (e.g. avoid USR for macros).
-llvm::Optional getSymbolID(const llvm::StringRef MacroName,
- const MacroInfo *MI,
- const SourceManager &SM);
+SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
+ const SourceManager &SM);
 
 /// Returns a QualType as string. The result doesn't contain unwritten scopes
 /// like anonymous/inline namespace.

diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index 8396de635135..24678a3cc1bc 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -503,20 +503,19 @@ struct CodeCompletionBuilder {
 };
 
 // Determine the symbol ID for a Sema code completion result, if possible.
-llvm::Optional getSymbolID(const CodeCompletionResult &R,
- const SourceManager &SM) {
+SymbolID getSymbolID(const CodeCompletionResult &R, const SourceManager &SM) {
   switch (R.Kind) {
   case CodeCompletionResult::RK_Declaration:
   case CodeCompletionResult::RK_Pattern: {
 // Computing USR caches linkage, which may change after code completion.
 if (hasUnstableLinkage(R.Declaration))
-  return llvm::None;
+  return {};
 return clang::clangd::getSymbolID(R.Declaration);
   }
   case

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 302236.

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

https://reviews.llvm.org/D17993

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CXX/except/except.spec/p14-ir.cpp
  clang/test/CodeGen/arm64-microsoft-arguments.cpp
  clang/test/CodeGen/attr-nomerge.cpp
  clang/test/CodeGen/no-builtin.cpp
  clang/test/CodeGen/temporary-lifetime.cpp
  clang/test/CodeGenCUDA/device-var-init.cu
  clang/test/CodeGenCXX/2011-12-19-init-list-ctor.cpp
  
clang/test/CodeGenCXX/RelativeVTablesABI/child-inheritted-from-parent-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-1.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-2.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/member-function-pointer.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/multiple-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-and-child-in-comdats.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/pass-byval-attributes.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/vbase-offset.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/virtual-function-call.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  clang/test/CodeGenCXX/amdgcn-automatic-variable.cpp
  clang/test/CodeGenCXX/amdgcn-func-arg.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
  clang/test/CodeGenCXX/apple-kext.cpp
  clang/test/CodeGenCXX/arm.cpp
  clang/test/CodeGenCXX/arm64-constructor-return.cpp
  clang/test/CodeGenCXX/array-default-argument.cpp
  clang/test/CodeGenCXX/atomicinit.cpp
  clang/test/CodeGenCXX/attr-disable-tail-calls.cpp
  clang/test/CodeGenCXX/attr-target-mv-member-funcs.cpp
  clang/test/CodeGenCXX/attr-target-mv-out-of-line-defs.cpp
  clang/test/CodeGenCXX/attr.cpp
  clang/test/CodeGenCXX/auto-variable-template.cpp
  clang/test/CodeGenCXX/blocks-cxx11.cpp
  clang/test/CodeGenCXX/blocks.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/CodeGenCXX/builtin_LINE.cpp
  clang/test/CodeGenCXX/catch-undef-behavior.cpp
  clang/test/CodeGenCXX/cfi-cross-dso.cpp
  clang/test/CodeGenCXX/conditional-gnu-ext.cpp
  clang/test/CodeGenCXX/const-init-cxx11.cpp
  clang/test/CodeGenCXX/constructor-destructor-return-this.cpp
  clang/test/CodeGenCXX/constructor-direct-call.cpp
  clang/test/CodeGenCXX/constructor-init.cpp
  clang/test/CodeGenCXX/constructors.cpp
  clang/test/CodeGenCXX/copy-constructor-elim-2.cpp
  clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
  clang/test/CodeGenCXX/cxx0x-delegating-ctors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-constructors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx11-initializer-array-new.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp
  clang/test/CodeGenCXX/cxx1y-init-captures.cpp
  clang/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
  clang/test/CodeGenCXX/cxx1z-copy-omission.cpp
  clang/test/CodeGenCXX/cxx1z-decomposition.cpp
  clang/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx1z-lambda-star-this.cpp
  clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp
  clang/test/CodeGenCXX/debug-info-ms-dtor-thunks.cpp
  clang/test/CodeGenCXX/default-arg-temps.cpp
  clang/test/CodeGenCXX/default-arguments.cpp
  clang/test/CodeGenCXX/delete.cpp
  clang/test/CodeGenCXX/derived-to-base-conv.cpp
  clang/test/CodeGenCXX/destructors.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
  clang/test/CodeGenCXX/dllexport-ctor-closure.cpp
  clang/test/CodeGenCXX/dllexport-members.cpp
  clang/test/CodeGenCXX/dllexport.cpp
  clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
  clang/test/CodeGenCXX/dllimport-members.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/duplicate-mangled-name.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/empty-nontrivially-copyable.cpp
  clang/test/CodeGenCXX/exceptions-seh-filter-captures.cpp
  clang/test/CodeGenCXX/exceptions-seh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/CodeGenCXX/float128-declarations.cpp
  clang/test/CodeGenCXX/float16-declarations.cpp
  clang/test/CodeGenCXX/global-dtor-no-atexit.cpp
  clang/test/CodeGenCXX/global-init.cpp
  clang/test/CodeGenCXX/goto.cpp
  clang/test/CodeGenCXX/hidden-dllimport.cpp
  clang/test/CodeGenCXX/implicit-co

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson marked an inline comment as done.
CJ-Johnson added a comment.

In D17993#2367447 , @rsmith wrote:

> Thanks! We should also have a test for the behavior when targeting the MS 
> ABI, where we sometimes don't emit the `nonnull dereferenceable` because the 
> "`this`" pointer might actually point outside the object, but otherwise I 
> think this is ready to go.
>
> Please can you also put together a patch for the release notes? This seems 
> worth mentioning there.

Done and done. Thanks!


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

https://reviews.llvm.org/D17993

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

[PATCH] D89670: [clangd] Store the containing symbol for refs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, LGTM!




Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:782
+
+EXPECT_EQ(Ref1->Container, Ref2->Container);
+  };

can you also assert containers here are non-null (and below)



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:803
+
+  AssertSameContainer("toplevel1", "toplevel2");
+  AssertSameContainer("classscope1", "classscope2");

nit: i would suggest writing these in the form of:

```
auto Container = [](StringRef RangeName){
  auto Ref = findRefWithRange(RangeName;
  ASSERT_TRUE(Ref);
  ASSERT_FALSE(Ref->Container.isNull());
  return Ref->Container;
};
EXPECT_EQ(Container("sym1"), Container("sym2"));
EXPECT_NE(Container("sym1"), Container("sym2"));
```

then you could update the ones above as:
```
EXPECT_EQ(Container("rangename"), findSymbol(Symbols, "symname"));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89670

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

[PATCH] D90291: [clangd] Add lit tests for remote index

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang-tools-extra/clangd/test/lit.site.cfg.py.in:11
 config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.python_executable = "@Python3_EXECUTABLE@"
 

Could this use `PYTHON_EXECUTABLE` instead? I see other tests using that, and 
Python3_EXECUTABLE might not always be set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90291

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

[PATCH] D90595: [clangd] Fix race in background index rebuild, where index could stay stale.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, jfb, arphaman.
Herald added a project: clang.
sammccall requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

The BackgroundQueue worker that finishes the last task is responsible for
calling OnIdle(). However calling OnIdle itself may be the "last task"
necessitating another call to OnIdle!

Consider:

  Main  +T1 +T2+T3
  Worker1  [---T1---]   [---T3---]
  Worker2   [---T2---][---Idle1---]

Idle1 starts running before T3 has started, let alone finished.
So it doesn't satisfy the goal that OnIdle runs after other work.

On the other hand, Worker1 won't run OnIdle after T3, as Idle1 is still running.
Therefore Worker2 needs to run OnIdle again (Idle2) once Idle1 completes.
This patch does that.

An example test flake due to this oversight:
http://lab.llvm.org:8011/#/builders/100/builds/721/steps/7/logs/FAIL__Clangd_Unit_Tests__BackgroundIndexTest_Confi


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90595

Files:
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/BackgroundQueue.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp


Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -7,6 +7,7 @@
 #include "TestTU.h"
 #include "index/Background.h"
 #include "index/BackgroundRebuild.h"
+#include "support/Threading.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -826,6 +827,41 @@
   }
 }
 
+TEST(BackgroundQueueTest, Idle) {
+  std::atomic TaskCount(0), IdleCount(0);
+  BackgroundQueue::Task Task([&] { ++TaskCount; });
+
+  // Set up a queue initially with 10 tasks.
+  AsyncTaskRunner ThreadPool;
+  BackgroundQueue Q;
+  for (unsigned I = 0; I < 10; ++I)
+Q.push(Task);
+  Notification Start;
+  for (unsigned I = 0; I < 5; ++I)
+ThreadPool.runAsync("worker", [&] {
+  Start.wait();
+  Q.work(/*OnIdle=*/[&] {
+// Should be nothing else running now, so this assertion isn't racy.
+EXPECT_EQ(10u + 2u * IdleCount, TaskCount);
+
+// The first two times the queue goes idle, add a couple more tasks.
+// This should caute OnIdle to run again.
+if (++IdleCount <= 2) {
+  Q.push(Task);
+  Q.push(Task);
+}
+  });
+});
+
+  Start.notify();
+  ASSERT_TRUE(Q.blockUntilIdleForTest(/*TimeoutSeconds=*/5));
+  Q.stop();
+  ASSERT_TRUE(ThreadPool.wait(timeoutSeconds(5)));
+
+  EXPECT_EQ(3u, IdleCount)
+  << "Queue exhausts original items, then the extra two (twice)";
+}
+
 TEST(BackgroundQueueTest, Progress) {
   using testing::AnyOf;
   BackgroundQueue::Stats S;
Index: clang-tools-extra/clangd/index/BackgroundQueue.cpp
===
--- clang-tools-extra/clangd/index/BackgroundQueue.cpp
+++ clang-tools-extra/clangd/index/BackgroundQueue.cpp
@@ -46,8 +46,11 @@
 {
   std::unique_lock Lock(Mu);
   ++Stat.Completed;
-  if (Stat.Active == 1 && Queue.empty()) {
-// We just finished the last item, the queue is going idle.
+  // If we just finished the last item, the queue is going idle.
+  // Loop in case more items get scheduled and completed during OnIdle().
+  // (We treat this as going idle a second time).
+  while (Stat.Active == 1 && Queue.empty() &&
+ Stat.LastIdle != Stat.Completed) {
 assert(ShouldStop || Stat.Completed == Stat.Enqueued);
 Stat.LastIdle = Stat.Completed;
 if (OnIdle) {
Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -100,7 +100,7 @@
   void boost(llvm::StringRef Tag, unsigned NewPriority);
 
   // Process items on the queue until the queue is stopped.
-  // If the queue becomes empty, OnIdle will be called (on one worker).
+  // Each time the queue becomes empty, OnIdle will be called (on one worker).
   void work(std::function OnIdle = nullptr);
 
   // Stop processing new tasks, allowing all work() calls to return soon.


Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -7,6 +7,7 @@
 #include "TestTU.h"
 #include "index/Background.h"
 #include "index/BackgroundRebuild.h"
+#include "support/Threading.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tool

[PATCH] D90116: [clangd] Escape Unicode characters to fix Windows builds

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D90116#2353440 , @kbobyrev wrote:

> Hmm, I see. From the looks of it, the solution for several projects would be
>
>   add_compile_options("$<$:/utf-8>")
>   add_compile_options("$<$:/utf-8>")
>
> But I'm not sure if it makes sense in our case and I don't see many 
> `add_compile_options` in LLVM. Also, I don't have a way to test it out.



- I brought up the environment to reproduce this problem and can confirm, that 
this solution with `add_compile_options` works.
- Adding  `-DCMAKE_CXX_FLAGS="/utf-8" -DCMAKE_C_FLAGS="/utf-8"` into cmake 
command also helps (so, maybe we can update build documentation for MSVC (here? 
https://llvm.org/docs/CMake.html#microsoft-visual-c ) instead of  changes 
inside `llvm/CMakeLists.txt`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90116

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

[PATCH] D90588: [clangd] NFC: Only pass ASTContext and TokenBuffer in getFoldingRanges API

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This doesn't feel quite right to me - we're going to need to get PP conditional 
regions, include blocks etc from the ParsedAST (they're not in ASTContext).
My sense is that we'll need a fairly random subset of ParsedAST, and so 
ParsedAST is a reasonable abstraction unless it's hard to produce for tests. 
But it isn't!

What's the motivation for this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90588

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

[PATCH] D80961: [clang][AST] Ignore template instantiations if not in AsIs mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D80961#2353384 , @steveire wrote:

> Many of the changes which were part of a previous iteration of this change 
> were related to the change of default behavior of matchers. As the default is 
> no longer changed, those changes fell away.

Indeed! Thank you for continuing on this. Aside from a few small nits, I think 
this LGTM.




Comment at: clang/include/clang/AST/ASTNodeTraverser.h:485
 
-for (const auto *Child : D->specializations())
-  dumpTemplateDeclSpecialization(Child);
+if (Traversal == TK_AsIs)
+  for (const auto *Child : D->specializations())

Any interest in using curly braces here for readability?



Comment at: clang/lib/AST/ASTDumper.cpp:132
 
-  for (const auto *Child : D->specializations())
-dumpTemplateDeclSpecialization(Child, DumpExplicitInst,
-   !D->isCanonicalDecl());
+  if (GetTraversalKind() == TK_AsIs)
+for (const auto *Child : D->specializations())

Any interest in using curly braces here for readability?



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:287-289
+  if (Finder->getASTContext().getParentMapContext().getTraversalKind() ==
+  TK_IgnoreUnlessSpelledInSource) {
+if (Finder->isMatchingInImplicitTemplateInstantiation()) {

You can combine these into a single `if` statement (and then elide the braces).



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:314
 
+  if (Finder->getASTContext().getParentMapContext().getTraversalKind() ==
+  TK_IgnoreUnlessSpelledInSource) {

Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961

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

[PATCH] D90599: Fix a leak in `ASTUnit::LoadFromCommandLine()` wehn compiler invocation fails

[Boris Staletic via Phabricator via cfe-commits]

bstaletic created this revision.
bstaletic added a reviewer: aprantl.
bstaletic added projects: clang, clang-c.
Herald added a subscriber: cfe-commits.
bstaletic requested review of this revision.

1. Here 

 we allocate buffers to hold the contents of unsaved files.
2. The `RemappedFiles` vector is then passed to 
`ASTUnit::LoadFromCommandLine()` as an `ArrayRef`, here 

3. In ASTUnit::LoadFromCommandLine() 

 we try to make a compiler invocation and exit early if that fails 
.
4. If we did exit early from that function, both Unit and UnitErr are nullptr 
,
 making the function exit without ever cleaning up the memory allocations from 
step 1.

Notes:

1. The function allocating all those `RemappedFile` objects is not actually the 
same as the one deallocating, in the happy case.

1.1. I did not figure out where exactly does it get deleted, because the above 
seems to be enough data for now.

2. My attempted solution is to iterate over remapped files and deallocate in 
this block 
,
 but it seems odd to deallocate through `ArrayRef`.

Fixes https://bugs.llvm.org/show_bug.cgi?id=47832


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90599

Files:
  clang/lib/Frontend/ASTUnit.cpp


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1758,7 +1758,11 @@
 CI = createInvocationFromCommandLine(
 llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
 if (!CI)
+{
+  for (const auto &RF : RemappedFiles)
+delete RF.second;
   return nullptr;
+}
   }
 
   // Override any files that need remapping


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1758,7 +1758,11 @@
 CI = createInvocationFromCommandLine(
 llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
 if (!CI)
+{
+  for (const auto &RF : RemappedFiles)
+delete RF.second;
   return nullptr;
+}
   }
 
   // Override any files that need remapping
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: [clang][AST] Ignore template instantiations if not in AsIs mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Oops, forgot to click the important bit to actually accept the review. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961

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

[clang-tools-extra] a07a2c8 - Use --use-color in run-clang-tidy.py

[Aaron Ballman via cfe-commits]

Author: David Sanders
Date: 2020-11-02T08:38:20-05:00
New Revision: a07a2c88d96cc48d7d381a0300565090c24b9770

URL: 
https://github.com/llvm/llvm-project/commit/a07a2c88d96cc48d7d381a0300565090c24b9770
DIFF: 
https://github.com/llvm/llvm-project/commit/a07a2c88d96cc48d7d381a0300565090c24b9770.diff

LOG: Use --use-color in run-clang-tidy.py

Now that clang-tidy supports the --use-color command line option, it's
a better user experience to use --use-color in run-clang-tidy.py and
preserving the colored output.

Added: 


Modified: 
clang-tools-extra/clang-tidy/tool/run-clang-tidy.py

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py 
b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index 7e23419cd916..313ecd2f9571 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -83,7 +83,7 @@ def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, 
build_path,
 header_filter, allow_enabling_alpha_checkers,
 extra_arg, extra_arg_before, quiet, config):
   """Gets a command line for clang-tidy."""
-  start = [clang_tidy_binary]
+  start = [clang_tidy_binary, '--use-color']
   if allow_enabling_alpha_checkers:
 start.append('-allow-enabling-analyzer-alpha-checkers')
   if header_filter is not None:



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

[PATCH] D90110: [clang-tidy] Use --use-color in run-clang-tidy.py

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

In D90110#2367596 , @dsanders11 wrote:

> No one has raised any concerns, so if someone with commit access wants to 
> commit this on my behalf it would be appreciated.. Thanks!

I've commit on your behalf in a07a2c88d96cc48d7d381a0300565090c24b9770 
, thank 
you for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90110

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

[PATCH] D90553: Rename CXXUnresolvedConstructExpr::arg_size for consistency

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rsmith.
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM with some minor linting nits. Thanks!




Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:1617
+TEST(ASTMatchersTest, ArgumentCountIs_CXXUnresolvedConstructExpr) {
+  auto Code = "template  struct S{}; template  void "
+  "x() { auto s = S(); }";

The lint warnings here are actually correct and should be addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90553

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

[PATCH] D90188: Add support for attribute 'using_if_exists'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:3801
+/// error.
+class UnresolvedUsingIfExistsDecl final : public NamedDecl {
+  UnresolvedUsingIfExistsDecl(DeclContext *DC, SourceLocation Loc,

Why is this inheriting from a `NamedDecl` rather than a `UsingDecl`? Given that 
this is a type of using declaration, I guess I would have expected it to appear 
as such in the AST hierarchy. For instance, should people using AST matchers be 
able to match one of these as a using declaration or are they so different 
semantically that they need to be sibling AST nodes?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:554
+def note_empty_using_if_exists_here : Note<
+  "using declaration annotated with using_if_exists here">;
 

Please add single quotes around the attribute name.



Comment at: clang/lib/Sema/SemaDecl.cpp:1177
 
+  if (auto *EmptyD = dyn_cast(FirstDecl)) {
+Diag(NameLoc, diag::err_use_of_empty_using_if_exists);

`const auto *`



Comment at: clang/lib/Sema/SemaExpr.cpp:3146
   if (!VD) {
+if (auto *EmptyD = dyn_cast(D)) {
+  Diag(Loc, diag::err_use_of_empty_using_if_exists);

rsmith wrote:
> I think this should instead be done by `DiagnoseUseOfDecl` / `CanUseDecl`. 
> All the code paths we care about already go through that.
`const auto *`



Comment at: clang/lib/Sema/SemaExprMember.cpp:1169
 
+  if (auto *EmptyD = dyn_cast(MemberDecl)) {
+Diag(MemberLoc, diag::err_use_of_empty_using_if_exists);

`const auto *`



Comment at: clang/test/SemaCXX/using-if-exists-attr.cpp:9
+
+// FIXME: Should we support the C++ spelling here?
+using NS::x [[clang::using_if_exists]]; // expected-error {{an attribute list 
cannot appear here}}

Mordante wrote:
> I would prefer to allow the C++ spelling, the square brackets are also 
> supported in C when using `-fdouble-square-bracket-attributes`. (Not too 
> useful in this case since C doesn't support namespaces.) But as mentioned 
> before I prefer the attribute before the `using ` declaration.
I'm not certain what this FIXME means given that it is using the C++ spelling 
there.



Comment at: clang/test/SemaCXX/using-if-exists-attr.cpp:16
+template 
+using template_alias UIE = NS::x; // expected-warning {{'using_if_exists' 
attribute only applies to named declarations, types, and value declarations}}
+

erik.pilkington wrote:
> ldionne wrote:
> > I assume the attribute is ignored on these declarations? Why is this not an 
> > error instead, since it's clearly a programmer mistake?
> Yeah, its just ignored. This is how clang behaves generally with any 
> mis-applied attribute. An error seems more appropriate to me too honestly, 
> but that seems like a separate policy change. Maybe @aaron.ballman knows more 
> about the history/justification for this behaviour?
It depends entirely on how you write the `Subjects` line in `Attr.td` as to 
whether this will warn or err. By default, we warn because most attributes 
should be things you can ignore. But if the semantics of the attribute suggest 
that an error is a better approach, you can slap an `ErrorDiag` after the 
subject list to cause errors if the attribute is written on the wrong subject.


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

https://reviews.llvm.org/D90188

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

[clang] d28de0d - [Flang][Driver] Add PrintPreprocessedInput FrontendAction (`flang-new -E`)

[Andrzej Warzynski via cfe-commits]

Author: Caroline Concatto
Date: 2020-11-02T14:03:35Z
New Revision: d28de0d7f257bbe4703f3449f1672f1744cf306b

URL: 
https://github.com/llvm/llvm-project/commit/d28de0d7f257bbe4703f3449f1672f1744cf306b
DIFF: 
https://github.com/llvm/llvm-project/commit/d28de0d7f257bbe4703f3449f1672f1744cf306b.diff

LOG: [Flang][Driver] Add PrintPreprocessedInput FrontendAction (`flang-new -E`)

This patch implements the first frontend action for the Flang parser (i.e.
Fortran::parser). This action runs the preprocessor and is invoked with the
`-E` flag. (i.e. `flang-new -E ). The generated output is printed
to either stdout or the output file (specified with `-` or `-o `).

Note that currently there is no mechanism to map options for the
frontend driver (i.e. Fortran::frontend::FrontendOptions) to options for
the parser (i.e. Fortran::parser::Options). Instead,
Frotran::parser::options are hard-coded to:

```
std::vector searchDirectories{"."s};
searchDirectories = searchDirectories;
isFixedForm = false;
_encoding(Fortran::parser::Encoding::UTF_8);
```

These default settings are compatible with the current Flang driver. Further
work is required in order for CompilerInvocation to read and map
clang::driver::options to Fortran::parser::options.

Co-authored-by: Andrzej Warzynski 

Differential Revision: https://reviews.llvm.org/D88381

Added: 
flang/test/Frontend/Inputs/hello-world.c
flang/test/Frontend/print-preprocess-C-file.f90
flang/test/Frontend/print-preprocessed-file.f90
flang/unittests/Frontend/PrintPreprocessedTest.cpp

Modified: 
clang/include/clang/Driver/Options.td
flang/include/flang/Frontend/CompilerInstance.h
flang/include/flang/Frontend/CompilerInvocation.h
flang/include/flang/Frontend/FrontendActions.h
flang/include/flang/Frontend/FrontendOptions.h
flang/lib/Frontend/CompilerInstance.cpp
flang/lib/Frontend/CompilerInvocation.cpp
flang/lib/Frontend/FrontendAction.cpp
flang/lib/Frontend/FrontendActions.cpp
flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
flang/test/Flang-Driver/driver-help-hidden.f90
flang/test/Flang-Driver/driver-help.f90
flang/unittests/Frontend/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 165baf06cbb2..057c2606c69d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -391,7 +391,7 @@ def C : Flag<["-"], "C">, Flags<[CC1Option]>, 
Group,
 def D : JoinedOrSeparate<["-"], "D">, Group,
 Flags<[CC1Option]>, MetaVarName<"=">,
 HelpText<"Define  to  (or 1 if  omitted)">;
-def E : Flag<["-"], "E">, Flags<[NoXarchOption,CC1Option]>, 
Group,
+def E : Flag<["-"], "E">, Flags<[NoXarchOption,CC1Option, FlangOption, 
FC1Option]>, Group,
 HelpText<"Only run the preprocessor">;
 def F : JoinedOrSeparate<["-"], "F">, Flags<[RenderJoined,CC1Option]>,
 HelpText<"Add directory to framework include search path">;

diff  --git a/flang/include/flang/Frontend/CompilerInstance.h 
b/flang/include/flang/Frontend/CompilerInstance.h
index b3f77896b8cb..21ef9f0dcd47 100644
--- a/flang/include/flang/Frontend/CompilerInstance.h
+++ b/flang/include/flang/Frontend/CompilerInstance.h
@@ -10,12 +10,10 @@
 
 #include "flang/Frontend/CompilerInvocation.h"
 #include "flang/Frontend/FrontendAction.h"
+#include "flang/Parser/parsing.h"
 #include "flang/Parser/provenance.h"
 #include "llvm/Support/raw_ostream.h"
 
-#include 
-#include 
-
 namespace Fortran::frontend {
 
 class CompilerInstance {
@@ -26,6 +24,10 @@ class CompilerInstance {
   /// Flang file  manager.
   std::shared_ptr allSources_;
 
+  std::shared_ptr allCookedSources_;
+
+  std::shared_ptr parsing_;
+
   /// The diagnostics engine instance.
   llvm::IntrusiveRefCntPtr diagnostics_;
 
@@ -75,6 +77,13 @@ class CompilerInstance {
 
   bool HasAllSources() const { return allSources_ != nullptr; }
 
+  /// }
+  /// @name Parser Operations
+  /// {
+
+  /// Return parsing to be used by Actions.
+  Fortran::parser::Parsing &parsing() const { return *parsing_; }
+
   /// }
   /// @name High-Level Operations
   /// {

diff  --git a/flang/include/flang/Frontend/CompilerInvocation.h 
b/flang/include/flang/Frontend/CompilerInvocation.h
index 223047c2afbd..12915c7c1d9c 100644
--- a/flang/include/flang/Frontend/CompilerInvocation.h
+++ b/flang/include/flang/Frontend/CompilerInvocation.h
@@ -9,6 +9,7 @@
 #define LLVM_FLANG_FRONTEND_COMPILERINVOCATION_H
 
 #include "flang/Frontend/FrontendOptions.h"
+#include "flang/Parser/parsing.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "llvm/Option/ArgList.h"
@@ -40,15 +41,25 @@ class CompilerInvocationBase {
 };
 
 class CompilerInvocation : public CompilerInvocationBase {
-  /// Options controlling the frontend itself.
+  /// Options for the frontend driver
+  // TODO: Merge with or translate to

[PATCH] D88381: [Flang][Driver] Add PrintPreprocessed FrontendAction

[Andrzej Warzynski via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd28de0d7f257: [Flang][Driver] Add PrintPreprocessedInput 
FrontendAction (`flang-new -E`) (authored by CarolineConcatto, committed by 
awarzynski).

Changed prior to commit:
  https://reviews.llvm.org/D88381?vs=301066&id=302263#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88381

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendAction.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Frontend/Inputs/hello-world.c
  flang/test/Frontend/print-preprocess-C-file.f90
  flang/test/Frontend/print-preprocessed-file.f90
  flang/unittests/Frontend/CMakeLists.txt
  flang/unittests/Frontend/PrintPreprocessedTest.cpp

Index: flang/unittests/Frontend/PrintPreprocessedTest.cpp
===
--- /dev/null
+++ flang/unittests/Frontend/PrintPreprocessedTest.cpp
@@ -0,0 +1,79 @@
+//===- unittests/Frontend/PrintPreprocessedTest.cpp  FrontendAction tests--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+#include "flang/Frontend/CompilerInstance.h"
+#include "flang/Frontend/FrontendOptions.h"
+#include "flang/FrontendTool/Utils.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace Fortran::frontend;
+
+namespace {
+
+TEST(FrontendAction, PrintPreprocessedInput) {
+  std::string inputFile = "test-file.f";
+  std::error_code ec;
+
+  // 1. Create the input file for the file manager
+  // AllSources (which is used to manage files inside every compiler instance),
+  // works with paths. This means that it requires a physical file. Create one.
+  std::unique_ptr os{
+  new llvm::raw_fd_ostream(inputFile, ec, llvm::sys::fs::OF_None)};
+  if (ec)
+FAIL() << "Fail to create the file need by the test";
+
+  // Populate the input file with the pre-defined input and flush it.
+  *(os) << "! test-file.F:\n"
+<< "#ifdef NEW\n"
+<< "  Program A \n"
+<< "#else\n"
+<< "  Program B\n"
+<< "#endif";
+  os.reset();
+
+  // Get the path of the input file
+  llvm::SmallString<64> cwd;
+  if (std::error_code ec = llvm::sys::fs::current_path(cwd))
+FAIL() << "Failed to obtain the current working directory";
+  std::string testFilePath(cwd.c_str());
+  testFilePath += "/" + inputFile;
+
+  // 2. Prepare the compiler (CompilerInvocation + CompilerInstance)
+  CompilerInstance compInst;
+  compInst.CreateDiagnostics();
+  auto invocation = std::make_shared();
+  invocation->frontendOpts().programAction_ = PrintPreprocessedInput;
+
+  compInst.set_invocation(std::move(invocation));
+  compInst.frontendOpts().inputs_.push_back(
+  FrontendInputFile(testFilePath, Language::Fortran));
+
+  // 3. Set-up the output stream. Using output buffer wrapped as an output
+  // stream, as opposed to an actual file (or a file descriptor).
+  llvm::SmallVector outputFileBuffer;
+  std::unique_ptr outputFileStream(
+  new llvm::raw_svector_ostream(outputFileBuffer));
+  compInst.set_outputStream(std::move(outputFileStream));
+
+  // 4. Run the earlier defined FrontendAction
+  bool success = ExecuteCompilerInvocation(&compInst);
+
+  // 5. Validate the expected output
+  EXPECT_TRUE(success);
+  EXPECT_TRUE(!outputFileBuffer.empty());
+  EXPECT_TRUE(
+  llvm::StringRef(outputFileBuffer.data()).startswith("program b\n"));
+
+  // 6. Clear the input and the output files. Since we used an output buffer,
+  // there are no physical output files to delete.
+  llvm::sys::fs::remove(inputFile);
+  compInst.ClearOutputFiles(/*EraseFiles=*/true);
+}
+} // namespace
Index: flang/unittests/Frontend/CMakeLists.txt
===
--- flang/unittests/Frontend/CMakeLists.txt
+++ flang/unittests/Frontend/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_flang_unittest(FlangFrontendTests
   CompilerInstanceTest.cpp
   InputOutputTest.cpp
+  PrintPreprocessedTest.cpp
 )
 
 target_link_libraries(FlangFrontendTests
@@ -10,4 +11,4 @@
   flangFrontend
   flangFrontendTool
   FortranParser
-  )
+)
Index: flang/test/Frontend/prin

[PATCH] D88381: [Flang][Driver] Add PrintPreprocessed FrontendAction

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D88381#2357123 , @sameeranjoshi 
wrote:

> I would wait for a couple of more days for someone to review from community 
> may be from Nvidia's side if someone would verify the initial design.

As there were no new reviews, I assumed that this good to land and have just 
merged it. I encourage people to submit post-commit reviews if they identify 
any issues with this patch that we've missed.

Thank you all for reviewing!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88381

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

[PATCH] D89790: [clangd] Add basic conflict detection for the rename.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:263
+  DeclContextLookupResult LookupResult;
+  switch (DC->getDeclKind()) {
+  case Decl::TranslationUnit:

explain this list somewhat?
e.g. these are the declcontexts which form a namespace where conflicts can 
occur?
and why function doesn't belong here



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:263
+  DeclContextLookupResult LookupResult;
+  switch (DC->getDeclKind()) {
+  case Decl::TranslationUnit:

sammccall wrote:
> explain this list somewhat?
> e.g. these are the declcontexts which form a namespace where conflicts can 
> occur?
> and why function doesn't belong here
I think you want to walk up DC while isTransparentContext()



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:315
+  // conflicts if we perform the rename.
+  // (!) DeclContext::lookup doesn't perform lookup local decls in a
+  // function-kind DeclContext.

Can you explain this a bit more (e.g. why?)

My guess is given:

```
void foo() {
  int bar;
}
```

`foo` is a DC but `bar` is not declared directly within it (rather within the 
CompoundStmt that is its body), therefore will not be looked up.

In which case an explanation like "Note that the enclosing DeclContext may not 
be the enclosing scope, particularly for function-local declarations, so this 
has both false positives and negatives." might help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89790

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

[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D90526#2367782 , @sammccall wrote:

> So I can't remember anymore why I put this as an enum `-log=public` rather 
> than an independent switch `-log-public` or so that could combine with any 
> log level.
> Sounds like the latter would be better?

Agreed.

> Manually insert it into the format string. After thinking about it I came to 
> the conclusion there just aren't that many places where we need to log things 
> publicly. I expect to basically not have any visibility inside most requests 
> except the error patterns.

SG.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90526

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

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D87449#2358579 , @balazske wrote:

> I think the name of this checker should be changed. It could in future not 
> only check for the SIG30-C rule. (Plan is to include C++ checks too, and 
> SIG31-C could be checked in this checker too.) It can be called 
> "bugprone-signal-handler" instead?

I have no issues putting this into the `bugprone` module and then aliasing to 
it from the `cert` module.




Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33
+
+  for (const FunctionDecl *D : Node.redecls())
+if (D->getASTContext().getSourceManager().isInSystemHeader(

balazske wrote:
> balazske wrote:
> > aaron.ballman wrote:
> > > balazske wrote:
> > > > aaron.ballman wrote:
> > > > > balazske wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > balazske wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > I'm not certain I understand why we're looking through the 
> > > > > > > > > entire redeclaration chain to see if the function is ever 
> > > > > > > > > mentioned in a system header. I was expecting we'd look at 
> > > > > > > > > the expansion location of the declaration and see if that's 
> > > > > > > > > in a system header, which is already handled by the 
> > > > > > > > > `isExpansionInSystemHeader()` matcher. Similar below.
> > > > > > > > This function is called from ` SignalHandlerCheck::check` when 
> > > > > > > > any function call is found. So the check for system header is 
> > > > > > > > needed. It was unclear to me what the "expansion location" 
> > > > > > > > means but it seems to work if using that expansion location and 
> > > > > > > > checking for system header, instead of this loop. I will update 
> > > > > > > > the code.
> > > > > > > > This function is called from  SignalHandlerCheck::check when 
> > > > > > > > any function call is found. So the check for system header is 
> > > > > > > > needed. 
> > > > > > > 
> > > > > > > The check for the system header isn't what I was concerned by, it 
> > > > > > > was the fact that we're walking the entire redeclaration chain to 
> > > > > > > see if *any* declaration is in a system header that I don't 
> > > > > > > understand the purpose of.
> > > > > > > 
> > > > > > > > It was unclear to me what the "expansion location" means but it 
> > > > > > > > seems to work if using that expansion location and checking for 
> > > > > > > > system header, instead of this loop. I will update the code.
> > > > > > > 
> > > > > > > The spelling location is where the user wrote the code and the 
> > > > > > > expansion location is where the macro name is written, but 
> > > > > > > thinking on it harder, that shouldn't matter for this situation 
> > > > > > > as either location would be in the system header anyway.
> > > > > > The function declaration is not often a macro name so there is no 
> > > > > > "expansion location" or the same as the original location. My 
> > > > > > concern was that if there is a declaration of system call function 
> > > > > > in a source file (like `void abort(void);` in .c file) for any 
> > > > > > reason, we may find this declaration instead of the one in the 
> > > > > > header file, if not looping over the declaration chain.
> > > > > > The function declaration is not often a macro name so there is no 
> > > > > > "expansion location" or the same as the original location.
> > > > > 
> > > > > Agreed.
> > > > > 
> > > > > > My concern was that if there is a declaration of system call 
> > > > > > function in a source file (like `void abort(void);` in .c file) for 
> > > > > > any reason, we may find this declaration instead of the one in the 
> > > > > > header file, if not looping over the declaration chain.
> > > > > 
> > > > > Typically, when a C user does something like that, it's because 
> > > > > they're explicitly *not* including the header file at all (they're 
> > > > > just forward declaring the function so they can use it) and so we'd 
> > > > > get the logic wrong for them anyway because we'd never find the 
> > > > > declaration in the system header.
> > > > > 
> > > > > Using the canonical declaration is more typical and would 
> > > > > realistically fail only in circumstances like forward declaring a 
> > > > > system header function and then later including the system header 
> > > > > itself. That said, I suppose your approach is defensible enough. 
> > > > > Redeclaration chains are hopefully short enough that it isn't a 
> > > > > performance hit.
> > > > I changed back to the original code to search the entire redeclaration 
> > > > chain. Otherwise it can be fooled with declarations before or after 
> > > > inclusion of the system header. Such declarations were added to the 
> > > > test file (it did not pass if `isExpansionInSystemHeader` was used).
> > > I don't think that's necessary (we aren't consistent about whether we 
> > > check the entire redecl 

[PATCH] D90291: [clangd] Add lit tests for remote index

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/test/lit.site.cfg.py.in:11
 config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.python_executable = "@Python3_EXECUTABLE@"
 

hans wrote:
> Could this use `PYTHON_EXECUTABLE` instead? I see other tests using that, and 
> Python3_EXECUTABLE might not always be set.
i was checking llvm's 
[lit.site.cfg.py.in](http://github.com//llvm/llvm-project/tree/master/llvm/test/lit.site.cfg.py.in)
 for reference and it was using python3 too.

any specific reasons for using PYTHON_EXECUTABLE instead of python3? I can't 
say this was a strong preference, but the python script included might not be 
python2 compliant, hence the need for 3.

If there is a compelling reason for using a possibly python2 executable, we can 
try to make the script both 2 and 3 compliant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90291

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

[PATCH] D90303: [ASTMatchers] Made isExpandedFromMacro Polymorphic

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:313
+  AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc),
+  std::string, MacroName) {
   // Verifies that the statement' beginning and ending are both expanded from

You mentioned that the change from `StringRef` to `std::string` was to avoid 
lifetime issues while matching, but I'm wondering if you can expound on that 
situation a bit more. I would have assumed that any memoization that involves 
`StringRef` would be responsible for the lifetime issues rather than the 
matchers themselves, but maybe I'm thinking about a different way you can hit 
lifetime issues than you are.



Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:150
+  )cc";
+  EXPECT_TRUE(matches(input, varDecl(isExpandedFromMacro("MY_MACRO";
+}

Can you also add a test for type-based matching?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90303

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

[PATCH] D90442: [PS4] Support dllimport/export attributes

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGff2e24a741e4: [PS4] Support dllimport/export attributes 
(authored by Ben Dunbobbin ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90442

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/ps4-dllimport-dllexport.c
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -782,6 +782,9 @@
 return isOSBinFormatXCOFF() || isWasm();
   }
 
+  /// Tests if the environment supports dllimport/export annotations.
+  bool hasDLLImportExport() const { return isOSWindows() || isPS4CPU(); }
+
   /// @}
   /// @name Mutators
   /// @{
Index: clang/test/CodeGen/ps4-dllimport-dllexport.c
===
--- /dev/null
+++ clang/test/CodeGen/ps4-dllimport-dllexport.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 \
+// RUN: -triple x86_64-scei-ps4 \
+// RUN: -fdeclspec \
+// RUN: -Werror \
+// RUN: -emit-llvm %s -o - | \
+// RUN:   FileCheck %s
+
+__declspec(dllexport) int export_int;
+
+__declspec(dllimport) int import_int;
+
+__declspec(dllexport) void export_declared_function();
+
+__declspec(dllexport) void export_implemented_function() {
+}
+
+__declspec(dllimport) void import_function(int);
+
+void call_imported_function() {
+  export_declared_function();
+  return import_function(import_int);
+}
+
+// CHECK-DAG: @import_int = external dllimport
+// CHECK-DAG: @export_int = dllexport global i32 0
+// CHECK-DAG: define dllexport void @export_implemented_function()
+// CHECK-DAG: declare dllimport void @import_function(i32)
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -368,8 +368,8 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
-def TargetWindows : TargetArch<["x86", "x86_64", "arm", "thumb", "aarch64"]> {
-  let OSes = ["Win32"];
+def TargetHasDLLImportExport : TargetSpec {
+  let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
 def TargetItaniumCXXABI : TargetSpec {
   let CustomCode = [{ Target.getCXXABI().isItaniumFamily() }];
@@ -3144,24 +3144,24 @@
   let SimpleHandler = 1;
 }
 
-def DLLExport : InheritableAttr, TargetSpecificAttr {
+def DLLExport : InheritableAttr, TargetSpecificAttr {
   let Spellings = [Declspec<"dllexport">, GCC<"dllexport">];
   let Subjects = SubjectList<[Function, Var, CXXRecord, ObjCInterface]>;
   let Documentation = [DLLExportDocs];
 }
 
-def DLLExportStaticLocal : InheritableAttr, TargetSpecificAttr {
+def DLLExportStaticLocal : InheritableAttr, TargetSpecificAttr {
   // This attribute is used internally only when -fno-dllexport-inlines is
-  // passed. This attribute is added to inline function of class having
-  // dllexport attribute. And if the function has static local variables, this
-  // attribute is used to whether the variables are exported or not. Also if
-  // function has local static variables, the function is dllexported too.
+  // passed. This attribute is added to inline functions of a class having the
+  // dllexport attribute. If the function has static local variables, this
+  // attribute is used to determine whether the variables are exported or not. If
+  // the function has local static variables, the function is dllexported too.
   let Spellings = [];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [Undocumented];
 }
 
-def DLLImport : InheritableAttr, TargetSpecificAttr {
+def DLLImport : InheritableAttr, TargetSpecificAttr {
   let Spellings = [Declspec<"dllimport">, GCC<"dllimport">];
   let Subjects = SubjectList<[Function, Var, CXXRecord, ObjCInterface]>;
   let Documentation = [DLLImportDocs];
@@ -3177,11 +3177,11 @@
   }];
 }
 
-def DLLImportStaticLocal : InheritableAttr, TargetSpecificAttr {
+def DLLImportStaticLocal : InheritableAttr, TargetSpecificAttr {
   // This attribute is used internally only when -fno-dllexport-inlines is
-  // passed. This attribute is added to inline function of class having
-  // dllimport attribute. And if the function has static local variables, this
-  // attribute is used to whether the variables are imported or not.
+  // passed. This attribute is added to inline functions of a class having the
+  // dllimport attribute. If the function has static local variables, this
+  // attribute is used to determine whether the variables are imported or not.
   let Spellings = [];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [Undocumente

[clang] ff2e24a - [PS4] Support dllimport/export attributes

[Ben Dunbobbin via cfe-commits]

Author: Ben Dunbobbin
Date: 2020-11-02T14:25:34Z
New Revision: ff2e24a741e4cee903fa71ec5c8c1909a89f66a3

URL: 
https://github.com/llvm/llvm-project/commit/ff2e24a741e4cee903fa71ec5c8c1909a89f66a3
DIFF: 
https://github.com/llvm/llvm-project/commit/ff2e24a741e4cee903fa71ec5c8c1909a89f66a3.diff

LOG: [PS4] Support dllimport/export attributes

For PS4 development we support dllimport/export annotations in
source code. This patch enables the dllimport/export attributes
on PS4 by adding a new function to query the triple for whether
dllimport/export are used and using that function to decide
whether these attributes are supported. This replaces the current
method of checking if the target is Windows.

This means we can drop the use of "TargetArch" in the .td file
(which is an improvement as dllimport/export support isn't really
a function of the architecture).

I have included a simple codgen test to show that the attributes
are accepted and have an effect on codegen for PS4. I have also
enabled the DLLExportStaticLocal and DLLImportStaticLocal
attributes, which we support downstream. However, I am unable to
write a test for these attributes until other patches for PS4
dllimport/export handling land upstream. Whilst writing this
patch I noticed that, as these attributes are internal, they do
not need to be target specific (when these attributes are added
internally in Clang the target specific checks have already been
run); however, I think leaving them target specific is fine
because it isn't harmful and they "really are" target specific
even if that has no functional impact.

Differential Revision: https://reviews.llvm.org/D90442

Added: 
clang/test/CodeGen/ps4-dllimport-dllexport.c

Modified: 
clang/include/clang/Basic/Attr.td
llvm/include/llvm/ADT/Triple.h

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 2b51a3ffcc6e..687c03f55841 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -368,8 +368,8 @@ def TargetRISCV : TargetArch<["riscv32", "riscv64"]>;
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
-def TargetWindows : TargetArch<["x86", "x86_64", "arm", "thumb", "aarch64"]> {
-  let OSes = ["Win32"];
+def TargetHasDLLImportExport : TargetSpec {
+  let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
 def TargetItaniumCXXABI : TargetSpec {
   let CustomCode = [{ Target.getCXXABI().isItaniumFamily() }];
@@ -3144,24 +3144,24 @@ def MSStruct : InheritableAttr {
   let SimpleHandler = 1;
 }
 
-def DLLExport : InheritableAttr, TargetSpecificAttr {
+def DLLExport : InheritableAttr, TargetSpecificAttr {
   let Spellings = [Declspec<"dllexport">, GCC<"dllexport">];
   let Subjects = SubjectList<[Function, Var, CXXRecord, ObjCInterface]>;
   let Documentation = [DLLExportDocs];
 }
 
-def DLLExportStaticLocal : InheritableAttr, TargetSpecificAttr {
+def DLLExportStaticLocal : InheritableAttr, 
TargetSpecificAttr {
   // This attribute is used internally only when -fno-dllexport-inlines is
-  // passed. This attribute is added to inline function of class having
-  // dllexport attribute. And if the function has static local variables, this
-  // attribute is used to whether the variables are exported or not. Also if
-  // function has local static variables, the function is dllexported too.
+  // passed. This attribute is added to inline functions of a class having the
+  // dllexport attribute. If the function has static local variables, this
+  // attribute is used to determine whether the variables are exported or not. 
If
+  // the function has local static variables, the function is dllexported too.
   let Spellings = [];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [Undocumented];
 }
 
-def DLLImport : InheritableAttr, TargetSpecificAttr {
+def DLLImport : InheritableAttr, TargetSpecificAttr {
   let Spellings = [Declspec<"dllimport">, GCC<"dllimport">];
   let Subjects = SubjectList<[Function, Var, CXXRecord, ObjCInterface]>;
   let Documentation = [DLLImportDocs];
@@ -3177,11 +3177,11 @@ public:
   }];
 }
 
-def DLLImportStaticLocal : InheritableAttr, TargetSpecificAttr {
+def DLLImportStaticLocal : InheritableAttr, 
TargetSpecificAttr {
   // This attribute is used internally only when -fno-dllexport-inlines is
-  // passed. This attribute is added to inline function of class having
-  // dllimport attribute. And if the function has static local variables, this
-  // attribute is used to whether the variables are imported or not.
+  // passed. This attribute is added to inline functions of a class having the
+  // dllimport attribute. If the function has static local variables, this
+  // attribute is used to determine whether the variables are imported or not.
   let Spellings = [];
   le

[PATCH] D90291: [clangd] Add lit tests for remote index

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang-tools-extra/clangd/test/lit.site.cfg.py.in:11
 config.host_triple = "@LLVM_HOST_TRIPLE@"
+config.python_executable = "@Python3_EXECUTABLE@"
 

kadircet wrote:
> hans wrote:
> > Could this use `PYTHON_EXECUTABLE` instead? I see other tests using that, 
> > and Python3_EXECUTABLE might not always be set.
> i was checking llvm's 
> [lit.site.cfg.py.in](http://github.com//llvm/llvm-project/tree/master/llvm/test/lit.site.cfg.py.in)
>  for reference and it was using python3 too.
> 
> any specific reasons for using PYTHON_EXECUTABLE instead of python3? I can't 
> say this was a strong preference, but the python script included might not be 
> python2 compliant, hence the need for 3.
> 
> If there is a compelling reason for using a possibly python2 executable, we 
> can try to make the script both 2 and 3 compliant.
> i was checking llvm's lit.site.cfg.py.in for reference and it was using 
> python3 too.

Oh interesting, I hadn't seen that. Well then using it here too makes sense I 
suppose. Sorry for the noise :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90291

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

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: llvm/docs/LangRef.rst:19494
+
+  declare void @llvm.ubsantrap(i32 immarg) cold noreturn nounwind
+

This is still i32.


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

https://reviews.llvm.org/D89959

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

[PATCH] D90042: [clang-tidy] performance-unnecessary-copy-initialization: Check for const reference arguments that are replaced template parameter type.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D90042#2360042 , @flx wrote:

> In D90042#2357078 , @aaron.ballman 
> wrote:
>
>> In D90042#2356265 , @flx wrote:
>>
>>> In D90042#2356180 , @aaron.ballman 
>>> wrote:
>>>
 In D90042#2350035 , @flx wrote:

> I should note that I was only able to reproduce the false positive with 
> the actual implementation std::function and not our fake version here.

 Any reason not to lift enough of the actual definition to be able to 
 reproduce the issue in your test cases? Does the change in definitions 
 break other tests?
>>>
>>> I poured over the actual definition and couldn't find any difference wrt 
>>> the call operator that would explain it. I would also think that:
>>>
>>>   template 
>>>   void foo(T&& t) {
>>> std::forward(t).modify();
>>>   }
>>>
>>> would be a simpler case that should trigger replacement, but it doesn't. Do 
>>> you have any idea what I could be missing?
>>
>> Perhaps silly question, but are you instantiating `foo()`?
>
> I think I added a full implementation of foo now, reverted the change, but am 
> still not getting the negative case to fail. Can you spot an issue with the 
> code?

I can't, but to be honest, I'm not certain I understand how that false positive 
could happen in the first place. That's why I was hoping to see the original 
case -- one thing you could try is with the original code, pass `-E` to 
preprocess to a file, and then try reducing the test case from that output 
(either by hand or by using a tool like creduce), or did you already give that 
a shot?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90042

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

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added a comment.

Is anything planned or in progress that would address this issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1435
+  let Spellings = [GCC<"leaf">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

Should this attribute also be supported on things like ObjC method decls or 
other function-like interfaces?



Comment at: clang/include/clang/Basic/Attr.td:1436
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;

This should be referring to `LeafDocs` rather than undocumented.



Comment at: clang/include/clang/Basic/AttrDocs.td:3909
+The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis 
in library functions.
+Functions marked as ``leaf`` attribute are not allowed to enter their caller's 
translation unit.
+Therefore, they cannot use or modify any data that does not escape the current 
compilation unit.

as leaf -> with the leaf

I'm not certain how to interpret that functions are not allowed to enter their 
caller's translation unit. I sort of read that as leaf functions are not 
allowed to call (or otherwise jump) out of the translation unit in which 
they're defined -- is that about right?



Comment at: clang/include/clang/Basic/AttrDocs.td:3910
+Functions marked as ``leaf`` attribute are not allowed to enter their caller's 
translation unit.
+Therefore, they cannot use or modify any data that does not escape the current 
compilation unit.
+

What sort of diagnostic checking should the user expect? e.g., can the user 
expect Clang to diagnose obviously incorrect code like:
```
[[gnu::leaf]] void func(void (*fp)(void)) {
  fp(); // This seems like a bad thing
}
```



Comment at: clang/include/clang/Basic/AttrDocs.td:3913
+For more information see
+`gcc documentation 
`
+}];

We should probably link to the latest docs 
(https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html) rather 
than 4.7.2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

In D72218#2364297 , @ffrankies wrote:

> @aaron.ballman Can you please commit this on my behalf? My github username is 
> ffrankies .

I've commit on your behalf in 43a38a65233039b5e71797a644d41a890f8d7f2b 
, thank 
you!

> And could you take a look at D72241 altera single work item barrier check 
> ? It's also been updated and awaiting review.

I'll take a look, thank you for pinging it.


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

https://reviews.llvm.org/D72218

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

[clang-tools-extra] 43a38a6 - Add a new altera kernel name restriction check to clang-tidy.

[Aaron Ballman via cfe-commits]

Author: Frank Derry Wanye
Date: 2020-11-02T10:11:38-05:00
New Revision: 43a38a65233039b5e71797a644d41a890f8d7f2b

URL: 
https://github.com/llvm/llvm-project/commit/43a38a65233039b5e71797a644d41a890f8d7f2b
DIFF: 
https://github.com/llvm/llvm-project/commit/43a38a65233039b5e71797a644d41a890f8d7f2b.diff

LOG: Add a new altera kernel name restriction check to clang-tidy.

The altera kernel name restriction check finds kernel files and include
directives whose filename is "kernel.cl", "Verilog.cl", or "VHDL.cl".
Such kernel file names cause the Altera Offline Compiler to generate
intermediate design files that have the same names as certain internal
files, which leads to a compilation error.

As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA
SDK for OpenCL Pro Edition: Programming Guide."

Added: 
clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
clang-tools-extra/docs/clang-tidy/checks/altera-kernel-name-restriction.rst

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/kernel.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/verilog.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/vhdl.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vERILOG.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl

clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp

Modified: 
clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
clang-tools-extra/clang-tidy/altera/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp 
b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
index d91f67ac1485..d3e906b673ce 100644
--- a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "KernelNameRestrictionCheck.h"
 #include "StructPackAlignCheck.h"
 
 using namespace clang::ast_matchers;
@@ -20,6 +21,8 @@ namespace altera {
 class AlteraModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"altera-kernel-name-restriction");
 CheckFactories.registerCheck(
 "altera-struct-pack-align");
   }

diff  --git a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
index ed28d9f4892d..8ab5cc1aa4ad 100644
--- a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_clang_library(clangTidyAlteraModule
   AlteraTidyModule.cpp
+  KernelNameRestrictionCheck.cpp
   StructPackAlignCheck.cpp
 
   LINK_LIBS

diff  --git 
a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp 
b/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
new file mode 100644
index ..eb49977cbedb
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
@@ -0,0 +1,107 @@
+//===--- KernelNameRestrictionCheck.cpp - clang-tidy 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt 

[clang-tools-extra] 5a7bc5e - Fix link to a new check within the release notes.

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2020-11-02T10:19:24-05:00
New Revision: 5a7bc5e2595903e51f0b31e3faf82024e965c962

URL: 
https://github.com/llvm/llvm-project/commit/5a7bc5e2595903e51f0b31e3faf82024e965c962
DIFF: 
https://github.com/llvm/llvm-project/commit/5a7bc5e2595903e51f0b31e3faf82024e965c962.diff

LOG: Fix link to a new check within the release notes.

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a5f1502f51c8..d29c0730ed6d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -86,7 +86,7 @@ New checks
 ^^
 
 - New :doc:`altera-kernel-name-restriction
-  ` check.
+  ` check.
 
   Finds kernel files and include directives whose filename is `kernel.cl`,
   `Verilog.cl`, or `VHDL.cl`.



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

[clang-tools-extra] c883904 - Revert "Add a new altera kernel name restriction check to clang-tidy."

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2020-11-02T10:30:42-05:00
New Revision: c88390468cdd1de4283b2c5c7889f16c477791cd

URL: 
https://github.com/llvm/llvm-project/commit/c88390468cdd1de4283b2c5c7889f16c477791cd
DIFF: 
https://github.com/llvm/llvm-project/commit/c88390468cdd1de4283b2c5c7889f16c477791cd.diff

LOG: Revert "Add a new altera kernel name restriction check to clang-tidy."

This reverts commit 43a38a65233039b5e71797a644d41a890f8d7f2b,
and follow-up 5a7bc5e2595903e51f0b31e3faf82024e965c962.

The commit breaks check-clang-tools, the test added in the change
does not pass.

Added: 


Modified: 
clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
clang-tools-extra/clang-tidy/altera/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 
clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
clang-tools-extra/docs/clang-tidy/checks/altera-kernel-name-restriction.rst

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/kernel.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/verilog.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/vhdl.cl/foo.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vERILOG.cl

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl

clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp



diff  --git a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp 
b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
index d3e906b673ce..d91f67ac1485 100644
--- a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
@@ -9,7 +9,6 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
-#include "KernelNameRestrictionCheck.h"
 #include "StructPackAlignCheck.h"
 
 using namespace clang::ast_matchers;
@@ -21,8 +20,6 @@ namespace altera {
 class AlteraModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
-CheckFactories.registerCheck(
-"altera-kernel-name-restriction");
 CheckFactories.registerCheck(
 "altera-struct-pack-align");
   }

diff  --git a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
index 8ab5cc1aa4ad..ed28d9f4892d 100644
--- a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
@@ -5,7 +5,6 @@ set(LLVM_LINK_COMPONENTS
 
 add_clang_library(clangTidyAlteraModule
   AlteraTidyModule.cpp
-  KernelNameRestrictionCheck.cpp
   StructPackAlignCheck.cpp
 
   LINK_LIBS

diff  --git 
a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp 
b/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
deleted file mode 100644
index eb49977cbedb..
--- a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
+++ /dev/null
@@ -1,107 +0,0 @@
-//===--- KernelNameRestrictionCheck.cpp - clang-tidy 
--===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "KernelNameRestrictionCheck.h"
-#include "clang/Frontend/C

Re: [clang-tools-extra] 43a38a6 - Add a new altera kernel name restriction check to clang-tidy.

[Nico Weber via cfe-commits]

The test in this commit doesn't pass. I've reverted this for now
in c88390468cdd1de4283b2c5c7889f16c477791cd

Please run the tests you're adding before committing a change.

On Mon, Nov 2, 2020 at 10:12 AM Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Frank Derry Wanye
> Date: 2020-11-02T10:11:38-05:00
> New Revision: 43a38a65233039b5e71797a644d41a890f8d7f2b
>
> URL:
> https://github.com/llvm/llvm-project/commit/43a38a65233039b5e71797a644d41a890f8d7f2b
> DIFF:
> https://github.com/llvm/llvm-project/commit/43a38a65233039b5e71797a644d41a890f8d7f2b.diff
>
> LOG: Add a new altera kernel name restriction check to clang-tidy.
>
> The altera kernel name restriction check finds kernel files and include
> directives whose filename is "kernel.cl", "Verilog.cl", or "VHDL.cl".
> Such kernel file names cause the Altera Offline Compiler to generate
> intermediate design files that have the same names as certain internal
> files, which leads to a compilation error.
>
> As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA
> SDK for OpenCL Pro Edition: Programming Guide."
>
> Added:
> clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
> clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
>
> clang-tools-extra/docs/clang-tidy/checks/altera-kernel-name-restriction.rst
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/
> kernel.cl
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/
> vhdl.cl
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/
> otherthing.cl
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/
> kernel.cl
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/
> kernel.cl/foo.h
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/
> verilog.cl/foo.h
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/
> vhdl.cl/foo.h
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/
> some_kernel.cl
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/
> verilog.cl
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vERILOG.cl
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h
>
> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/
> vhdl_number_two.cl
>
> clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
>
> Modified:
> clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
> clang-tools-extra/clang-tidy/altera/CMakeLists.txt
> clang-tools-extra/docs/ReleaseNotes.rst
> clang-tools-extra/docs/clang-tidy/checks/list.rst
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
> b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
> index d91f67ac1485..d3e906b673ce 100644
> --- a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
> +++ b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
> @@ -9,6 +9,7 @@
>  #include "../ClangTidy.h"
>  #include "../ClangTidyModule.h"
>  #include "../ClangTidyModuleRegistry.h"
> +#include "KernelNameRestrictionCheck.h"
>  #include "StructPackAlignCheck.h"
>
>  using namespace clang::ast_matchers;
> @@ -20,6 +21,8 @@ namespace altera {
>  class AlteraModule : public ClangTidyModule {
>  public:
>void addCheckFactories(ClangTidyCheckFactories &CheckFactories)
> override {
> +CheckFactories.registerCheck(
> +"altera-kernel-name-restriction");
>  CheckFactories.registerCheck(
>  "altera-struct-pack-align");
>}
>
> diff  --git a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
> b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
> index ed28d9f4892d..8ab5cc1aa4ad 100644
> --- a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
> +++ b/clang-tools-extra/clang-tidy/altera/CMakeLists.txt
> @@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
>
>  add_clang_library(clangTidyAlteraModule
>AlteraTidyModule.cpp
> +  KernelNameRestrictionCheck.cpp
>StructPackAlignCheck.cpp
>
>LINK_LIBS
>
> diff  --git
> a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
> b/clang-tools-extra/clang

Re: [clang-tools-extra] 43a38a6 - Add a new altera kernel name restriction check to clang-tidy.

[Aaron Ballman via cfe-commits]

On Mon, Nov 2, 2020 at 10:32 AM Nico Weber  wrote:
>
> The test in this commit doesn't pass. I've reverted this for now in 
> c88390468cdd1de4283b2c5c7889f16c477791cd

Thanks for the revert. Can you link to a bot for the failure? The only
failure I saw was with the documentation building and I had already
corrected it.

> Please run the tests you're adding before committing a change.

FWIW, I did and they passed for me locally, but perhaps something is
different about my setup (or perhaps I was in a wonky state, etc)?

~Aaron

> On Mon, Nov 2, 2020 at 10:12 AM Aaron Ballman via cfe-commits 
>  wrote:
>>
>>
>> Author: Frank Derry Wanye
>> Date: 2020-11-02T10:11:38-05:00
>> New Revision: 43a38a65233039b5e71797a644d41a890f8d7f2b
>>
>> URL: 
>> https://github.com/llvm/llvm-project/commit/43a38a65233039b5e71797a644d41a890f8d7f2b
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/43a38a65233039b5e71797a644d41a890f8d7f2b.diff
>>
>> LOG: Add a new altera kernel name restriction check to clang-tidy.
>>
>> The altera kernel name restriction check finds kernel files and include
>> directives whose filename is "kernel.cl", "Verilog.cl", or "VHDL.cl".
>> Such kernel file names cause the Altera Offline Compiler to generate
>> intermediate design files that have the same names as certain internal
>> files, which leads to a compilation error.
>>
>> As per the "Guidelines for Naming the Kernel" section in the "Intel FPGA
>> SDK for OpenCL Pro Edition: Programming Guide."
>>
>> Added:
>> clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
>> clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
>> 
>> clang-tools-extra/docs/clang-tidy/checks/altera-kernel-name-restriction.rst
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/kernel.cl/foo.h
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/verilog.cl/foo.h
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/vhdl.cl/foo.h
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vERILOG.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h
>> 
>> clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl
>> 
>> clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
>>
>> Modified:
>> clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
>> clang-tools-extra/clang-tidy/altera/CMakeLists.txt
>> clang-tools-extra/docs/ReleaseNotes.rst
>> clang-tools-extra/docs/clang-tidy/checks/list.rst
>>
>> Removed:
>>
>>
>>
>> 
>> diff  --git a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp 
>> b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
>> index d91f67ac1485..d3e906b673ce 100644
>> --- a/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
>> +++ b/clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
>> @@ -9,6 +9,7 @@
>>  #include "../ClangTidy.h"
>>  #include "../ClangTidyModule.h"
>>  #include "../ClangTidyModuleRegistry.h"
>> +#include "KernelNameRestrictionCheck.h"
>>  #include "StructPackAlignCheck.h"
>>
>>  using namespace clang::ast_matchers;
>> @@ -20,6 +21,8 @@ namespace altera {
>>  class AlteraModule : public ClangTidyModule {
>>  public:
>>void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
>> +CheckFactories.registerCheck(
>> +"altera-kernel-name-restriction");
>>  CheckFactories.registerCheck(
>>  "altera-struct-pack-align");
>>}
>>
>> diff  --git a/clang-tools-extra/clang-tidy/altera/CMakeLists.txt 
>> b/clang-tools-extra/cla

[PATCH] D84362: [NFC] Refactor DiagnosticBuilder and PartialDiagnostic

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D84362#2365153 , @rjmccall wrote:

> I may not have been clear. I'm not saying SourceLocation is a meaningful 
> concept in the driver.

Likewise, I should be more careful with respect to how I refer to 
`SourceLocation` (a class in Clang) and "source location" (location within a 
source file).

I see your point. Basically, you are suggesting that we improve the diagnostics 
for the driver first. And for that we'd need something like 
`CommandLineLocation` (a new class: location within the command invocation) 
that would be related to `SourceLocation` via a common base class, e.g. 
`AbstractLocation` (also a new class). That would indeed be a very nice thing 
to have.

However, this is not what we've been proposing. Our ultimate goal is a new 
Flang driver:

- implemented in terms of libclangDriver
- independent of Clang.

We shouldn't need to implement this extra functionality to achieve our goal. 
Personally I like the idea, but we need to stay focused on activities that are 
strictly needed for a new Flang driver. Not making things worse for Clang is an 
equally important goal. Improving things in Clang is something we are keen on, 
but are unlikely to have the bandwidth for.

It'll hard to create these thin layers above the diagnostic classes that do not 
depend on `SourceLocation` and `SourceManager` (and other classes that 
depend/require these). This is regardless of whether driver diagnostics are 
capable of tracking location (via e.g. `CommandLineLocation`) or not. The 
integration of diagnostics classes and "source location" related classes 
(specifically `SourceLocation` and `SourceManager`) is quite deep and this 
patch deepens that. IMO this patch makes things even harder for us. I'm 
starting to think that all this could be vastly simplified if libclangDriver 
had it's own diagnostics classes- a very simplified version of what's currently 
available in libclangBasic (again, because the driver doesn't need much). The 
improvements that you propose could be added at later time. Would this be 
acceptable? I think that the thin abstraction layer that we're discussing here 
would naturally emerge with time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84362

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

[clang] 22e7182 - [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

[Kirstóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-11-02T16:41:17+01:00
New Revision: 22e7182002b5396539c69603b1c8c924b5f661e7

URL: 
https://github.com/llvm/llvm-project/commit/22e7182002b5396539c69603b1c8c924b5f661e7
DIFF: 
https://github.com/llvm/llvm-project/commit/22e7182002b5396539c69603b1c8c924b5f661e7.diff

LOG: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

ReturnPtrRange checker emits a report if a function returns a pointer which
points out of the buffer. However, end() iterator of containers is always such
a pointer, so this always results a false positive report. This false positive
case is now eliminated.

This patch resolves these tickets:
https://bugs.llvm.org/show_bug.cgi?id=20929
https://bugs.llvm.org/show_bug.cgi?id=25226
https://bugs.llvm.org/show_bug.cgi?id=27701

Patch by Tibor Brunner!

Differential Revision: https://reviews.llvm.org/D83678

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
clang/test/Analysis/misc-ps-region-store.m
clang/test/Analysis/return-ptr-range.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
index 599d4f306aa1..1a94ccdc2825 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
@@ -58,6 +58,11 @@ void ReturnPointerRangeChecker::checkPreStmt(const 
ReturnStmt *RS,
   DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
   state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType());
 
+  // We assume that the location after the last element in the array is used as
+  // end() iterator. Reporting on these would return too many false positives.
+  if (Idx == ElementCount)
+return;
+
   ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
   ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
   if (StOutBound && !StInBound) {
@@ -70,7 +75,7 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt 
*RS,
 // types explicitly reference such exploit categories (when applicable).
 if (!BT)
   BT.reset(new BuiltinBug(
-  this, "Return of pointer value outside of expected range",
+  this, "Buffer overflow",
   "Returned pointer value points outside the original object "
   "(potential buffer overflow)"));
 

diff  --git a/clang/test/Analysis/misc-ps-region-store.m 
b/clang/test/Analysis/misc-ps-region-store.m
index b8710ff6f638..9c31324e7e59 100644
--- a/clang/test/Analysis/misc-ps-region-store.m
+++ b/clang/test/Analysis/misc-ps-region-store.m
@@ -463,7 +463,7 @@ void element_region_with_symbolic_superregion(int* p) {
 
 static int test_cwe466_return_outofbounds_pointer_a[10];
 int *test_cwe466_return_outofbounds_pointer() {
-  int *p = test_cwe466_return_outofbounds_pointer_a+10;
+  int *p = test_cwe466_return_outofbounds_pointer_a+11;
   return p; // expected-warning{{Returned pointer value points outside the 
original object}}
 }
 

diff  --git a/clang/test/Analysis/return-ptr-range.cpp 
b/clang/test/Analysis/return-ptr-range.cpp
index dd5dcd5d5d19..9763b51264e6 100644
--- a/clang/test/Analysis/return-ptr-range.cpp
+++ b/clang/test/Analysis/return-ptr-range.cpp
@@ -25,3 +25,47 @@ int *test_element_index_lifetime_with_local_ptr() {
   } while (0);
   return local_ptr; // expected-warning{{Returned pointer value points outside 
the original object (potential buffer overflow)}}
 }
+
+template 
+T* end(T (&arr)[N]) {
+  return arr + N; // no-warning, because we want to avoid false positives on 
returning the end() iterator of a container.
+}
+
+void get_end_of_array() {
+  static int arr[10];
+  end(arr);
+}
+
+template 
+class Iterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  Iterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish; }
+};
+
+void use_iterable_object() {
+  Iterable<20> iter;
+  iter.end();
+}
+
+template 
+class BadIterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  BadIterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish + 1; } // expected-warning{{Returned pointer 
value points outside the original object (potential buffer overflow)}}
+};
+
+void use_bad_iterable_object() {
+  BadIterable<20> iter;
+  iter.end();
+}



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

[PATCH] D83678: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

[Kristóf Umann via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG22e7182002b5: [analyzer][ReturnPtrRangeChecker] Fix a false 
positive on end() iterator (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D83678?vs=287881&id=302279#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83678

Files:
  clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/return-ptr-range.cpp


Index: clang/test/Analysis/return-ptr-range.cpp
===
--- clang/test/Analysis/return-ptr-range.cpp
+++ clang/test/Analysis/return-ptr-range.cpp
@@ -25,3 +25,47 @@
   } while (0);
   return local_ptr; // expected-warning{{Returned pointer value points outside 
the original object (potential buffer overflow)}}
 }
+
+template 
+T* end(T (&arr)[N]) {
+  return arr + N; // no-warning, because we want to avoid false positives on 
returning the end() iterator of a container.
+}
+
+void get_end_of_array() {
+  static int arr[10];
+  end(arr);
+}
+
+template 
+class Iterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  Iterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish; }
+};
+
+void use_iterable_object() {
+  Iterable<20> iter;
+  iter.end();
+}
+
+template 
+class BadIterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  BadIterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish + 1; } // expected-warning{{Returned pointer 
value points outside the original object (potential buffer overflow)}}
+};
+
+void use_bad_iterable_object() {
+  BadIterable<20> iter;
+  iter.end();
+}
Index: clang/test/Analysis/misc-ps-region-store.m
===
--- clang/test/Analysis/misc-ps-region-store.m
+++ clang/test/Analysis/misc-ps-region-store.m
@@ -463,7 +463,7 @@
 
 static int test_cwe466_return_outofbounds_pointer_a[10];
 int *test_cwe466_return_outofbounds_pointer() {
-  int *p = test_cwe466_return_outofbounds_pointer_a+10;
+  int *p = test_cwe466_return_outofbounds_pointer_a+11;
   return p; // expected-warning{{Returned pointer value points outside the 
original object}}
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
@@ -58,6 +58,11 @@
   DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
   state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType());
 
+  // We assume that the location after the last element in the array is used as
+  // end() iterator. Reporting on these would return too many false positives.
+  if (Idx == ElementCount)
+return;
+
   ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
   ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
   if (StOutBound && !StInBound) {
@@ -70,7 +75,7 @@
 // types explicitly reference such exploit categories (when applicable).
 if (!BT)
   BT.reset(new BuiltinBug(
-  this, "Return of pointer value outside of expected range",
+  this, "Buffer overflow",
   "Returned pointer value points outside the original object "
   "(potential buffer overflow)"));
 


Index: clang/test/Analysis/return-ptr-range.cpp
===
--- clang/test/Analysis/return-ptr-range.cpp
+++ clang/test/Analysis/return-ptr-range.cpp
@@ -25,3 +25,47 @@
   } while (0);
   return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
 }
+
+template 
+T* end(T (&arr)[N]) {
+  return arr + N; // no-warning, because we want to avoid false positives on returning the end() iterator of a container.
+}
+
+void get_end_of_array() {
+  static int arr[10];
+  end(arr);
+}
+
+template 
+class Iterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  Iterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish; }
+};
+
+void use_iterable_object() {
+  Iterable<20> iter;
+  iter.end();
+}
+
+template 
+class BadIterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  BadIterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish + 1; } // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+};
+
+void use_bad_iterable_object() {
+  BadIterable<20> i

[PATCH] D90618: chained diff

[Mikhail Goncharov via Phabricator via cfe-commits]

goncharov created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
goncharov requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90618

Files:
  clang-tools-extra/docs/new_file.txt


Index: clang-tools-extra/docs/new_file.txt
===
--- clang-tools-extra/docs/new_file.txt
+++ clang-tools-extra/docs/new_file.txt
@@ -1,3 +1,4 @@
 aaa
 ddd
 ccc
+eee


Index: clang-tools-extra/docs/new_file.txt
===
--- clang-tools-extra/docs/new_file.txt
+++ clang-tools-extra/docs/new_file.txt
@@ -1,3 +1,4 @@
 aaa
 ddd
 ccc
+eee
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

I might've missed it, but the debug location merging behavior could use a test. 
Here's one way to write it: https://godbolt.org/z/Yb9PY9.

In `@_Z13keep_locationPi`, the !dbg attachment on the trap should match 
`!DILocation(line: 1`. In `@_Z15merge_locationsPi`, the attachment should match 
`!DILocation(line: 0` (it currently incorrectly points to line 4).


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

https://reviews.llvm.org/D89959

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

[PATCH] D89980: [hip] Remove kernel argument coercion.

[Matt Arsenault via Phabricator via cfe-commits]

arsenm requested changes to this revision.
arsenm added a comment.
This revision now requires changes to proceed.

I think this is a dead end approach. I don't see the connection to the original 
problem you are trying to solve. Can you send me an IR testcase that this is 
supposed to help?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89980

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

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp:34
+ hasName("barrier"),
+ hasName("work_group_barrier")
+.bind("barrier")),

You can use `hasAnyName()` and pass both strings rather than two `hasName()` 
calls.



Comment at: 
clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp:38
+  unless(hasDescendant(callExpr(callee(functionDecl(anyOf(
+  hasName("get_global_id"), hasName("get_local_id")
+  .bind("function"),

Same here.



Comment at: 
clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp:49
+  bool IsNDRange = false;
+  for (const Attr *Attribute : MatchedDecl->getAttrs()) {
+if (Attribute->getKind() == attr::Kind::ReqdWorkGroupSize) {

Rather than getting all attributes, you can use `specific_attrs<>` to loop over 
just the specific attributes you care about on the declaration. That also fixes 
the non-idiomatic way to convert from an attribute to a specific derived 
attribute (which should use `dyn_cast<>` or friends).



Comment at: 
clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp:61
+diag(MatchedDecl->getLocation(),
+ "Kernel function %0 does not call get_global_id or get_local_id and "
+ "will be treated as single-work-item.\nBarrier call at %1 may error "

clang-tidy diagnostics shouldn't start with a capital letter or contain 
terminating punctuation, the function names should be surrounded in single 
quotes, and we don't typically use newlines or print out source locations like 
that in diagnostics. How about:

`kernel function %0 does not call 'get_global_id' or 'get_local_id' and is 
treated as a single work-item`

and issue a note diagnostic `barrier call may error out` at the barrier 
location?



Comment at: 
clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.cpp:71
+diag(MatchedDecl->getLocation(),
+ "Kernel function %0 does not call get_global_id or get_local_id may "
+ "be a viable single work-item kernel, but barrier call at %1 will "

Same here as above.

Will users know what `NDRange execution` means? (I'm can't really understand 
what the diagnostic is telling me, but this is outside of my typical domain.)



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/altera-single-work-item-barrier.rst:7
+Finds OpenCL kernel functions that call a barrier function but do not call
+an ID function.
+

Maybe link to documentation describing what an ID function is (or describe it 
more explicitly here)?



Comment at: 
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn:16
 "AlteraTidyModule.cpp",
+"SingleWorkItemBarrierCheck.cpp",
 "StructPackAlignCheck.cpp",

FWIW, you don't need to maintain the gn files and can leave them out of your 
patch.


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

https://reviews.llvm.org/D72241

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

[PATCH] D87194: Thread safety analysis: Use access specifiers to decide about scope

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Analysis/ThreadSafety.cpp:1283
+  return true;
+// We are ignoring friends here.
 if (!CurrentMethod)

This should probably have a FIXME?



Comment at: clang/lib/Analysis/ThreadSafety.cpp:1297-1299
+  for (const CXXBasePath &Path : Paths)
+if (Path.Access != AS_none)
+  return true;

`llvm::any_of()`?



Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:158
+}
+
 }  // end namespace ScopeTest

Given that this is touching on declaration contexts, it would also be good to 
have some tests checking for differences between declaration contexts (like 
out-of-line method definitions)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87194

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

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd updated this revision to Diff 302298.
compnerd added a comment.

rebase, clang-format, add comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88859

Files:
  clang/include/clang/APINotes/APINotesYAMLCompiler.h
  clang/include/clang/APINotes/Types.h
  clang/lib/APINotes/APINotesYAMLCompiler.cpp
  clang/lib/APINotes/CMakeLists.txt
  clang/lib/CMakeLists.txt
  clang/test/APINotes/Inputs/Frameworks/Simple.framework/Headers/Simple.apinotes
  clang/test/APINotes/Inputs/Frameworks/Simple.framework/Headers/Simple.h
  
clang/test/APINotes/Inputs/Frameworks/SimpleKit.framework/Headers/SimpleKit.apinotes
  clang/test/APINotes/Inputs/Frameworks/SimpleKit.framework/Headers/SimpleKit.h
  
clang/test/APINotes/Inputs/Frameworks/SimpleKit.framework/Headers/module.modulemap
  clang/test/APINotes/yaml-roundtrip-2.test
  clang/test/APINotes/yaml-roundtrip.test
  clang/test/CMakeLists.txt
  clang/test/lit.cfg.py
  clang/tools/CMakeLists.txt
  clang/tools/apinotes-test/APINotesTest.cpp
  clang/tools/apinotes-test/CMakeLists.txt

Index: clang/tools/apinotes-test/CMakeLists.txt
===
--- /dev/null
+++ clang/tools/apinotes-test/CMakeLists.txt
@@ -0,0 +1,6 @@
+set(LLVM_LINK_COMPONENTS
+  Support)
+add_clang_executable(apinotes-test
+  APINotesTest.cpp)
+clang_target_link_libraries(apinotes-test PRIVATE
+  clangAPINotes)
Index: clang/tools/apinotes-test/APINotesTest.cpp
===
--- /dev/null
+++ clang/tools/apinotes-test/APINotesTest.cpp
@@ -0,0 +1,53 @@
+//===-- APINotesTest.cpp - API Notes Testing Tool -- C++ --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/APINotes/APINotesYAMLCompiler.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/WithColor.h"
+
+static llvm::cl::list
+APINotes(llvm::cl::Positional, llvm::cl::desc("[ ...]"),
+ llvm::cl::Required);
+
+static llvm::cl::opt
+OutputFileName("o", llvm::cl::desc("output filename"),
+   llvm::cl::value_desc("filename"), llvm::cl::init("-"));
+
+int main(int argc, const char **argv) {
+  const bool DisableCrashReporting = true;
+  llvm::sys::PrintStackTraceOnErrorSignal(argv[0], DisableCrashReporting);
+  llvm::cl::ParseCommandLineOptions(argc, argv);
+
+  auto Error = [](const llvm::Twine &Msg) {
+llvm::WithColor::error(llvm::errs(), "apinotes-test") << Msg << '\n';
+  };
+
+  std::error_code EC;
+  auto Out = std::make_unique(OutputFileName, EC,
+llvm::sys::fs::OF_None);
+  if (EC) {
+Error("failed to open '" + OutputFileName + "': " + EC.message());
+return EXIT_FAILURE;
+  }
+
+  for (const std::string &Notes : APINotes) {
+llvm::ErrorOr> NotesOrError =
+llvm::MemoryBuffer::getFileOrSTDIN(Notes);
+if (std::error_code EC = NotesOrError.getError()) {
+  llvm::errs() << EC.message() << '\n';
+  return EXIT_FAILURE;
+}
+
+clang::api_notes::parseAndDumpAPINotes((*NotesOrError)->getBuffer(),
+   Out->os());
+  }
+
+  return EXIT_SUCCESS;
+}
Index: clang/tools/CMakeLists.txt
===
--- clang/tools/CMakeLists.txt
+++ clang/tools/CMakeLists.txt
@@ -2,6 +2,7 @@
 
 add_clang_subdirectory(diagtool)
 add_clang_subdirectory(driver)
+add_clang_subdirectory(apinotes-test)
 add_clang_subdirectory(clang-diff)
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -63,7 +63,8 @@
 tool_dirs = [config.clang_tools_dir, config.llvm_tools_dir]
 
 tools = [
-'c-index-test', 'clang-diff', 'clang-format', 'clang-tblgen', 'opt', 'llvm-ifs',
+'apinotes-test', 'c-index-test', 'clang-diff', 'clang-format',
+'clang-tblgen', 'opt', 'llvm-ifs',
 ToolSubst('%clang_extdef_map', command=FindTool(
 'clang-extdef-mapping'), unresolved='ignore'),
 ]
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -58,6 +58,7 @@
 endif ()
 
 list(APPEND CLANG_TEST_DEPS
+  apinotes-test
   c-index-test
   clang
   clang-resource-headers
Index: clang/test/APINotes/yaml-roundtrip.test
===
--- /dev/null
+++ clang/test/APINotes/yaml-roundtrip.test
@@ -0,0 +1,26 @@

[PATCH] D90622: clang: Don't assert on no_unique_address fields in @encode()

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rjmccall.
thakis requested review of this revision.

Just skip (non-bitfield) zero-sized fields, like we do with empty bases.

The class->struct conversion in the test is because -std=c++20 else deletes 
some default methods
due to non-accessible base dtors otherwise.

As a side-effect of writing the test, I discovered that D76801 
 did an ABI breaking change of sorts
for Objective-C's @encode. But it's been in for a while, so I'm not sure if we 
want to row back on
that or now.

Fixes PR48048.


https://reviews.llvm.org/D90622

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGenObjCXX/encode.mm


Index: clang/test/CodeGenObjCXX/encode.mm
===
--- clang/test/CodeGenObjCXX/encode.mm
+++ clang/test/CodeGenObjCXX/encode.mm
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=gnu++98 %s -triple=x86_64-apple-darwin10 -emit-llvm -o 
- | FileCheck %s
+// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++98 %s 
-triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck --check-prefixes 
CHECK,CHECKCXX98 %s
+// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++20 %s 
-triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck --check-prefixes 
CHECK,CHECKCXX20 %s
 
 // CHECK: v17@0:8{vector=}16
 // CHECK: {vector=}
@@ -87,7 +88,9 @@
 
   typedef vector< float,  fixed<4> > vector4f;
 
-  // CHECK: @_ZN11rdar93574002ggE = constant [49 x i8] c"{vector >=[4f]}\00"
+  // FIXME: This difference is due to D76801. It was probably an unintentional 
change. Maybe we want to undo it?
+  // CHECKCXX98: @_ZN11rdar93574002ggE = constant [49 x i8] c"{vector >=[4f]}\00"
+  // CHECKCXX20: @_ZN11rdar93574002ggE = constant [48 x i8] c"{vector>=[4f]}\00"
   extern const char gg[] = @encode(vector4f);
 }
 
@@ -170,21 +173,21 @@
 
 
 // PR10990
-class CefBase {
+struct CefBase {
   virtual ~CefBase() {}
 };
-class CefBrowser : public virtual CefBase {};
-class CefBrowserImpl : public CefBrowser {};
+struct CefBrowser : public virtual CefBase {};
+struct CefBrowserImpl : public CefBrowser {};
 // CHECK: @g6 = constant [21 x i8] c"{CefBrowserImpl=^^?}\00"
 extern const char g6[] = @encode(CefBrowserImpl);
 
 // PR10990_2
-class CefBase2 {
+struct CefBase2 {
   virtual ~CefBase2() {}
   int i;
 };
-class CefBrowser2 : public virtual CefBase2 {};
-class CefBrowserImpl2 : public CefBrowser2 {};
+struct CefBrowser2 : public virtual CefBase2 {};
+struct CefBrowserImpl2 : public CefBrowser2 {};
 // CHECK: @g7 = constant [26 x i8] c"{CefBrowserImpl2=^^?^^?i}\00"
 extern const char g7[] = @encode(CefBrowserImpl2);
 
@@ -245,3 +248,15 @@
   // CHECK: @{{.*}} = private unnamed_addr constant [13 x i8] 
c"{N={S=@}}\00"
   return @encode(N);
 }
+
+#if __cplusplus >= 202002L
+namespace PR48048 {
+  struct F {};
+  struct I {
+int m;
+[[no_unique_address]] F n;
+  };
+  // CHECKCXX20: @_ZN7PR480481xE = constant [6 x i8] c"{I=i}\00"
+  extern const char x[] = @encode(I);
+}
+#endif
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -7658,7 +7658,9 @@
   }
 
   unsigned i = 0;
-  for (auto *Field : RDecl->fields()) {
+  for (FieldDecl *Field : RDecl->fields()) {
+if (!Field->isZeroLengthBitField(*this) && Field->isZeroSize(*this))
+  continue;
 uint64_t offs = layout.getFieldOffset(i);
 FieldOrBaseOffsets.insert(FieldOrBaseOffsets.upper_bound(offs),
   std::make_pair(offs, Field));


Index: clang/test/CodeGenObjCXX/encode.mm
===
--- clang/test/CodeGenObjCXX/encode.mm
+++ clang/test/CodeGenObjCXX/encode.mm
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=gnu++98 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++98 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECKCXX98 %s
+// RUN: %clang_cc1 -Wno-objc-root-class -std=gnu++20 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECKCXX20 %s
 
 // CHECK: v17@0:8{vector=}16
 // CHECK: {vector=}
@@ -87,7 +88,9 @@
 
   typedef vector< float,  fixed<4> > vector4f;
 
-  // CHECK: @_ZN11rdar93574002ggE = constant [49 x i8] c"{vector >=[4f]}\00"
+  // FIXME: This difference is due to D76801. It was probably an unintentional change. Maybe we want to undo it?
+  // CHECKCXX98: @_ZN11rdar93574002ggE = constant [49 x i8] c"{vector >=[4f]}\00"
+  // CHECKCXX20: @_ZN11rdar93574002ggE = constant [48 x i8] c"{vector>=[4f]}\00"
   extern const char gg[] = @encode(vector4f);
 }
 
@@ -170,21 +173,21 @@
 
 
 // PR10990
-class CefBase {
+struct CefBase {
   virtual ~CefBase() {}
 };
-class CefBrowser : public virtual CefBase {};
-class CefBrowserImpl : public CefBrowser {};
+struct CefBrowser : public virtual CefBase {};
+struct CefBr

[PATCH] D90518: [clangd] Make tests depend on Clang

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai abandoned this revision.
smeenai added a comment.

Cool, thanks for fixing it better!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90518

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

[PATCH] D90528: [clangd] Fix check-clangd with no clang built

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90528

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

[PATCH] D90516: [clang] Limit scope of CLANG_VENDOR definition

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e4ab8e0152b: [clang] Limit scope of CLANG_VENDOR definition 
(authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90516

Files:
  clang/CMakeLists.txt
  clang/lib/Basic/CMakeLists.txt


Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -33,6 +33,11 @@
   PROPERTIES GENERATED TRUE
  HEADER_FILE_ONLY TRUE)
 
+if(CLANG_VENDOR)
+  set_source_files_properties(Version.cpp
+PROPERTIES COMPILE_DEFINITIONS "CLANG_VENDOR=\"${CLANG_VENDOR} \"")
+endif()
+
 add_clang_library(clangBasic
   Attributes.cpp
   Builtins.cpp
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -332,10 +332,6 @@
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 
-if( CLANG_VENDOR )
-  add_definitions( -DCLANG_VENDOR="${CLANG_VENDOR} " )
-endif()
-
 set(CLANG_REPOSITORY_STRING "" CACHE STRING
   "Vendor-specific text for showing the repository the source is taken from.")
 


Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -33,6 +33,11 @@
   PROPERTIES GENERATED TRUE
  HEADER_FILE_ONLY TRUE)
 
+if(CLANG_VENDOR)
+  set_source_files_properties(Version.cpp
+PROPERTIES COMPILE_DEFINITIONS "CLANG_VENDOR=\"${CLANG_VENDOR} \"")
+endif()
+
 add_clang_library(clangBasic
   Attributes.cpp
   Builtins.cpp
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -332,10 +332,6 @@
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 
-if( CLANG_VENDOR )
-  add_definitions( -DCLANG_VENDOR="${CLANG_VENDOR} " )
-endif()
-
 set(CLANG_REPOSITORY_STRING "" CACHE STRING
   "Vendor-specific text for showing the repository the source is taken from.")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4e4ab8e - [clang] Limit scope of CLANG_VENDOR definition

[Shoaib Meenai via cfe-commits]

Author: Shoaib Meenai
Date: 2020-11-02T09:04:43-08:00
New Revision: 4e4ab8e0152b42baef5e5a1e2484d865e1a57e90

URL: 
https://github.com/llvm/llvm-project/commit/4e4ab8e0152b42baef5e5a1e2484d865e1a57e90
DIFF: 
https://github.com/llvm/llvm-project/commit/4e4ab8e0152b42baef5e5a1e2484d865e1a57e90.diff

LOG: [clang] Limit scope of CLANG_VENDOR definition

It's only used by Version.cpp, so limit the definition to just that one
file instead of making all of Clang recompile if you change CLANG_VENDOR.

Reviewed By: phosek

Differential Revision: https://reviews.llvm.org/D90516

Added: 


Modified: 
clang/CMakeLists.txt
clang/lib/Basic/CMakeLists.txt

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index a2b99e2e37e9..8c539e80946d 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -332,10 +332,6 @@ set(CLANG_SYSTEMZ_DEFAULT_ARCH "z10" CACHE STRING "SystemZ 
Default Arch")
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 
-if( CLANG_VENDOR )
-  add_definitions( -DCLANG_VENDOR="${CLANG_VENDOR} " )
-endif()
-
 set(CLANG_REPOSITORY_STRING "" CACHE STRING
   "Vendor-specific text for showing the repository the source is taken from.")
 

diff  --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt
index d259b951f2c2..0ca4148801fc 100644
--- a/clang/lib/Basic/CMakeLists.txt
+++ b/clang/lib/Basic/CMakeLists.txt
@@ -33,6 +33,11 @@ set_source_files_properties("${version_inc}"
   PROPERTIES GENERATED TRUE
  HEADER_FILE_ONLY TRUE)
 
+if(CLANG_VENDOR)
+  set_source_files_properties(Version.cpp
+PROPERTIES COMPILE_DEFINITIONS "CLANG_VENDOR=\"${CLANG_VENDOR} \"")
+endif()
+
 add_clang_library(clangBasic
   Attributes.cpp
   Builtins.cpp



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

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D90180#2366418 , @trixirt wrote:

> I am trying to address problems treewide so around 100 changes per fix.
> Doing that requires consensus that the fix is generally ok for every 
> subsystem in the code base.
> So while clang will fix
> switch () {} ; 
> it will also fix
> for () {
>
>   // tbd : add something here
>   ;
>
> }
> consensus needs to be found on as narrow a fix as possible.
> so there will be a specialized fixer for each problem consensus is reached on.

Interesting approach, thank you for sharing it.

> As for leaving whitespace fixing as an exercise for the user.
> No change will be accepted with a whitespace problem.
> When there are 100 fixes, there are 100 files to open, find the line, fix the 
> line.
> This is slow and error prone.

It's also not really required. Apply the automated fixes, run the results 
through clang-format-diff.py 
(http://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting), 
and commit the results. If this is error-prone, we've got a pretty big issue 
(IMO).

> Another slow part is manually commit and submitting the fixes and doing a 
> review cycle.
> But if your fixer can automagically do everything you can  hook it into a c/i 
> and and
> it will keep the codebase free of its set of fixers while we all do something 
> more 
> interesting.
>
> i am working on the kernel build to do this now.
> It would be helpful if this and future fixers could live in clang-tidy's 
> linuxkernel/
> which is already part of the kernel build so i don't have to reinvent how to 
> find them.

Given that we already have a module for Linux kernel-specific clang-tidy checks 
for this to live in, I can hold my nose on the fact that it's poorly 
replicating the frontend's work because other users won't be surprised by it 
(in theory). So I'll retract my disapproval; you have compelling rationale for 
why you want to do it this way.

Since the plan is to produce multiple distinct checks for each extraneous 
semicolon situation, then I think this check should probably be renamed to 
something more generic and given options to control which distinct scenarios to 
check for. This will reduce the amount of compilation overhead for the 
clang-tidy project over time by not needing to introduce a new check (with new 
boilerplate) for each scenario but should hopefully still allow you to do what 
you need (with config files perhaps) in your CI. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90180

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

[clang-tools-extra] 6bd01b8 - [clangd] Account for vendor in version string

[Shoaib Meenai via cfe-commits]

Author: Shoaib Meenai
Date: 2020-11-02T09:04:44-08:00
New Revision: 6bd01b8184de93979756331c71af69b422f71103

URL: 
https://github.com/llvm/llvm-project/commit/6bd01b8184de93979756331c71af69b422f71103
DIFF: 
https://github.com/llvm/llvm-project/commit/6bd01b8184de93979756331c71af69b422f71103.diff

LOG: [clangd] Account for vendor in version string

The vendor will be prefixed to the "clangd" and can be an arbitrary
string, so account for it in the test.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D90517

Added: 


Modified: 
clang-tools-extra/clangd/test/log.test

Removed: 




diff  --git a/clang-tools-extra/clangd/test/log.test 
b/clang-tools-extra/clangd/test/log.test
index 68d6d29c6120..5d60c10eb917 100644
--- a/clang-tools-extra/clangd/test/log.test
+++ b/clang-tools-extra/clangd/test/log.test
@@ -1,5 +1,5 @@
 # RUN: env CLANGD_FLAGS=-index-file=no-such-index not clangd -lit-test 
&1 >/dev/null | FileCheck %s
-CHECK: I[{{.*}}] clangd version {{.*}}
+CHECK: I[{{.*}}]{{.*}} clangd version {{.*}}
 CHECK: Working directory: {{.*}}
 CHECK: argv[0]: clangd
 CHECK: argv[1]: -lit-test



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

[PATCH] D90517: [clangd] Account for vendor in version string

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6bd01b8184de: [clangd] Account for vendor in version string 
(authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90517

Files:
  clang-tools-extra/clangd/test/log.test


Index: clang-tools-extra/clangd/test/log.test
===
--- clang-tools-extra/clangd/test/log.test
+++ clang-tools-extra/clangd/test/log.test
@@ -1,5 +1,5 @@
 # RUN: env CLANGD_FLAGS=-index-file=no-such-index not clangd -lit-test 
&1 >/dev/null | FileCheck %s
-CHECK: I[{{.*}}] clangd version {{.*}}
+CHECK: I[{{.*}}]{{.*}} clangd version {{.*}}
 CHECK: Working directory: {{.*}}
 CHECK: argv[0]: clangd
 CHECK: argv[1]: -lit-test


Index: clang-tools-extra/clangd/test/log.test
===
--- clang-tools-extra/clangd/test/log.test
+++ clang-tools-extra/clangd/test/log.test
@@ -1,5 +1,5 @@
 # RUN: env CLANGD_FLAGS=-index-file=no-such-index not clangd -lit-test &1 >/dev/null | FileCheck %s
-CHECK: I[{{.*}}] clangd version {{.*}}
+CHECK: I[{{.*}}]{{.*}} clangd version {{.*}}
 CHECK: Working directory: {{.*}}
 CHECK: argv[0]: clangd
 CHECK: argv[1]: -lit-test
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 415f7ee - [Clang] Add the ability to map DLL storage class to visibility

[Ben Dunbobbin via cfe-commits]

Author: Ben Dunbobbin
Date: 2020-11-02T17:08:23Z
New Revision: 415f7ee8836944942d8beb70e982e95a312866a7

URL: 
https://github.com/llvm/llvm-project/commit/415f7ee8836944942d8beb70e982e95a312866a7
DIFF: 
https://github.com/llvm/llvm-project/commit/415f7ee8836944942d8beb70e982e95a312866a7.diff

LOG: [Clang] Add the ability to map DLL storage class to visibility

For PlayStation we offer source code compatibility with
Microsoft's dllimport/export annotations; however, our file
format is based on ELF.

To support this we translate from DLL storage class to ELF
visibility at the end of codegen in Clang.

Other toolchains have used similar strategies (e.g. see the
documentation for this ARM toolchain:

https://developer.arm.com/documentation/dui0530/i/migrating-from-rvct-v3-1-to-rvct-v4-0/changes-to-symbol-visibility-between-rvct-v3-1-and-rvct-v4-0)

This patch adds the ability to perform this translation. Options
are provided to support customizing the mapping behaviour.

Differential Revision: https://reviews.llvm.org/D89970

Added: 
clang/test/CodeGenCXX/visibility-dllstorageclass.cpp
clang/test/Driver/ps4-visibility-dllstorageclass.c
clang/test/Driver/visibility-dllstorageclass.c

Modified: 
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/PS4CPU.cpp
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index d711d66784a4..1d203f8489eb 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -307,6 +307,16 @@ ENUM_LANGOPT(TypeVisibilityMode, Visibility, 3, 
DefaultVisibility,
  "default visibility for types [-ftype-visibility]")
 LANGOPT(SetVisibilityForExternDecls, 1, 0,
 "apply global symbol visibility to external declarations without an 
explicit visibility")
+LANGOPT(VisibilityFromDLLStorageClass, 1, 0,
+"set the visiblity of globals from their DLL storage class 
[-fvisibility-from-dllstorageclass]")
+ENUM_LANGOPT(DLLExportVisibility, Visibility, 3, DefaultVisibility,
+ "visibility for functions and variables with dllexport 
annotations [-fvisibility-from-dllstorageclass]")
+ENUM_LANGOPT(NoDLLStorageClassVisibility, Visibility, 3, HiddenVisibility,
+ "visibility for functions and variables without an explicit DLL 
storage class [-fvisibility-from-dllstorageclass]")
+ENUM_LANGOPT(ExternDeclDLLImportVisibility, Visibility, 3, DefaultVisibility,
+ "visibility for external declarations with dllimport annotations 
[-fvisibility-from-dllstorageclass]")
+ENUM_LANGOPT(ExternDeclNoDLLStorageClassVisibility, Visibility, 3, 
HiddenVisibility,
+ "visibility for external declarations without an explicit DLL 
storage class [-fvisibility-from-dllstorageclass]")
 BENIGN_LANGOPT(SemanticInterposition, 1, 0, "semantic interposition")
 BENIGN_LANGOPT(ExplicitNoSemanticInterposition, 1, 0, "explicitly no semantic 
interposition")
 ENUM_LANGOPT(StackProtector, StackProtectorMode, 2, SSPOff,

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 057c2606c69d..33cfa72c0888 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1973,6 +1973,15 @@ def fno_var_tracking : Flag<["-"], "fno-var-tracking">, 
Group, Group,
   HelpText<"Generate verbose assembly output">;
 def dA : Flag<["-"], "dA">, Alias;
+defm visibility_from_dllstorageclass : 
OptInFFlag<"visibility-from-dllstorageclass", "Set the visiblity of symbols in 
the generated code from their DLL storage class">;
+def fvisibility_dllexport_EQ : Joined<["-"], "fvisibility-dllexport=">, 
Group, Flags<[CC1Option]>,
+  HelpText<"The visibility for dllexport defintions 
[-fvisibility-from-dllstorageclass]">, Values<"hidden,protected,default">;
+def fvisibility_nodllstorageclass_EQ : Joined<["-"], 
"fvisibility-nodllstorageclass=">, Group, Flags<[CC1Option]>,
+  HelpText<"The visibility for defintiions without an explicit DLL export 
class [-fvisibility-from-dllstorageclass]">, Values<"hidden,protected,default">;
+def fvisibility_externs_dllimport_EQ : Joined<["-"], 
"fvisibility-externs-dllimport=">, Group, Flags<[CC1Option]>,
+  HelpText<"The visibility for dllimport external declarations 
[-fvisibility-from-dllstorageclass]">, Values<"hidden,protected,default">;
+def fvisibility_externs_nodllstorageclass_EQ : Joined<["-"], 
"fvisibility-externs-nodllstorageclass=">, Group, Flags<[CC1Option]>,
+  HelpText<"The visibility for external declarations without an explicit DLL 
dllstorageclass [-fvisibility-from-dllstorageclass]">, 
Values<"hidden,protected,default">;
 def fvisibility_EQ : Joined<["-"], "fvisibilit

[PATCH] D89970: Add the ability to map DLL storage class to visibility

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG415f7ee88369: [Clang] Add the ability to map DLL storage 
class to visibility (authored by Ben Dunbobbin ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D89970?vs=301803&id=302305#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89970

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/PS4CPU.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/visibility-dllstorageclass.cpp
  clang/test/Driver/ps4-visibility-dllstorageclass.c
  clang/test/Driver/visibility-dllstorageclass.c

Index: clang/test/Driver/visibility-dllstorageclass.c
===
--- /dev/null
+++ clang/test/Driver/visibility-dllstorageclass.c
@@ -0,0 +1,87 @@
+// Check behaviour of -fvisibility-from-dllstorageclass options
+
+// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
+// RUN: -Werror -S -### %s 2>&1 | \
+// RUN:   FileCheck %s \
+// RUN: --implicit-check-not=-fvisibility-from-dllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-dllexport \
+// RUN: --implicit-check-not=-fvisibility-nodllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-externs-dllimport \
+// RUN: --implicit-check-not=-fvisibility-externs-nodllstorageclass
+
+// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
+// RUN: -fvisibility-from-dllstorageclass \
+// RUN: -fno-visibility-from-dllstorageclass \
+// RUN: -Werror -S -### %s 2>&1 | \
+// RUN:   FileCheck %s \
+// RUN: --implicit-check-not=-fvisibility-from-dllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-dllexport \
+// RUN: --implicit-check-not=-fvisibility-nodllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-externs-dllimport \
+// RUN: --implicit-check-not=-fvisibility-externs-nodllstorageclass
+
+// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
+// RUN: -fno-visibility-from-dllstorageclass \
+// RUN: -fvisibility-from-dllstorageclass \
+// RUN: -Werror -S -### %s 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=SET \
+// RUN: --implicit-check-not=-fvisibility-from-dllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-dllexport \
+// RUN: --implicit-check-not=-fvisibility-nodllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-externs-dllimport \
+// RUN: --implicit-check-not=-fvisibility-externs-nodllstorageclass
+
+// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
+// RUN: -fvisibility-dllexport=hidden \
+// RUN: -fvisibility-nodllstorageclass=protected \
+// RUN: -fvisibility-externs-dllimport=hidden \
+// RUN: -fvisibility-externs-nodllstorageclass=protected \
+// RUN: -S -### %s 2>&1 | \
+// RUN:   FileCheck %s --check-prefixes=UNUSED \
+// RUN: --implicit-check-not=-fvisibility-from-dllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-dllexport \
+// RUN: --implicit-check-not=-fvisibility-nodllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-externs-dllimport \
+// RUN: --implicit-check-not=-fvisibility-externs-nodllstorageclass \
+// RUN: --implicit-check-not=error: \
+// RUN: --implicit-check-not=warning:
+
+// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
+// RUN: -fno-visibility-from-dllstorageclass \
+// RUN: -fvisibility-dllexport=hidden \
+// RUN: -fvisibility-nodllstorageclass=protected \
+// RUN: -fvisibility-externs-dllimport=hidden \
+// RUN: -fvisibility-externs-nodllstorageclass=protected \
+// RUN: -S -### %s 2>&1 | \
+// RUN:   FileCheck %s --check-prefixes=UNUSED \
+// RUN: --implicit-check-not=-fvisibility-from-dllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-dllexport \
+// RUN: --implicit-check-not=-fvisibility-nodllstorageclass \
+// RUN: --implicit-check-not=-fvisibility-externs-dllimport \
+// RUN: --implicit-check-not=-fvisibility-externs-nodllstorageclass \
+// RUN: --implicit-check-not=error: \
+// RUN: --implicit-check-not=warning:
+
+// UNUSED:  clang: warning: argument unused during compilation: '-fvisibility-dllexport=hidden'
+// UNUSED-NEXT: clang: warning: argument unused during compilation: '-fvisibility-nodllstorageclass=protected'
+// UNUSED-NEXT: clang: warning: argument unused during compilation: '-fvisibility-externs-dllimport=hidden'
+// UNUSED-NEXT: clang: warning: argument unused during compilation: '-fvisibility-externs-nodllstorageclass=protected'
+
+// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
+//

[clang] 5024d3a - Revert "[Clang] Add the ability to map DLL storage class to visibility"

[Ben Dunbobbin via cfe-commits]

Author: Ben Dunbobbin
Date: 2020-11-02T17:33:54Z
New Revision: 5024d3aa1855d4c17c7e875048d5ad20b8b2d8ce

URL: 
https://github.com/llvm/llvm-project/commit/5024d3aa1855d4c17c7e875048d5ad20b8b2d8ce
DIFF: 
https://github.com/llvm/llvm-project/commit/5024d3aa1855d4c17c7e875048d5ad20b8b2d8ce.diff

LOG: Revert "[Clang] Add the ability to map DLL storage class to visibility"

This reverts commit 415f7ee8836944942d8beb70e982e95a312866a7.

The added tests were failing on the build bots!

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/PS4CPU.cpp
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 
clang/test/CodeGenCXX/visibility-dllstorageclass.cpp
clang/test/Driver/ps4-visibility-dllstorageclass.c
clang/test/Driver/visibility-dllstorageclass.c



diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 1d203f8489eb..d711d66784a4 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -307,16 +307,6 @@ ENUM_LANGOPT(TypeVisibilityMode, Visibility, 3, 
DefaultVisibility,
  "default visibility for types [-ftype-visibility]")
 LANGOPT(SetVisibilityForExternDecls, 1, 0,
 "apply global symbol visibility to external declarations without an 
explicit visibility")
-LANGOPT(VisibilityFromDLLStorageClass, 1, 0,
-"set the visiblity of globals from their DLL storage class 
[-fvisibility-from-dllstorageclass]")
-ENUM_LANGOPT(DLLExportVisibility, Visibility, 3, DefaultVisibility,
- "visibility for functions and variables with dllexport 
annotations [-fvisibility-from-dllstorageclass]")
-ENUM_LANGOPT(NoDLLStorageClassVisibility, Visibility, 3, HiddenVisibility,
- "visibility for functions and variables without an explicit DLL 
storage class [-fvisibility-from-dllstorageclass]")
-ENUM_LANGOPT(ExternDeclDLLImportVisibility, Visibility, 3, DefaultVisibility,
- "visibility for external declarations with dllimport annotations 
[-fvisibility-from-dllstorageclass]")
-ENUM_LANGOPT(ExternDeclNoDLLStorageClassVisibility, Visibility, 3, 
HiddenVisibility,
- "visibility for external declarations without an explicit DLL 
storage class [-fvisibility-from-dllstorageclass]")
 BENIGN_LANGOPT(SemanticInterposition, 1, 0, "semantic interposition")
 BENIGN_LANGOPT(ExplicitNoSemanticInterposition, 1, 0, "explicitly no semantic 
interposition")
 ENUM_LANGOPT(StackProtector, StackProtectorMode, 2, SSPOff,

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 33cfa72c0888..057c2606c69d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1973,15 +1973,6 @@ def fno_var_tracking : Flag<["-"], "fno-var-tracking">, 
Group, Group,
   HelpText<"Generate verbose assembly output">;
 def dA : Flag<["-"], "dA">, Alias;
-defm visibility_from_dllstorageclass : 
OptInFFlag<"visibility-from-dllstorageclass", "Set the visiblity of symbols in 
the generated code from their DLL storage class">;
-def fvisibility_dllexport_EQ : Joined<["-"], "fvisibility-dllexport=">, 
Group, Flags<[CC1Option]>,
-  HelpText<"The visibility for dllexport defintions 
[-fvisibility-from-dllstorageclass]">, Values<"hidden,protected,default">;
-def fvisibility_nodllstorageclass_EQ : Joined<["-"], 
"fvisibility-nodllstorageclass=">, Group, Flags<[CC1Option]>,
-  HelpText<"The visibility for defintiions without an explicit DLL export 
class [-fvisibility-from-dllstorageclass]">, Values<"hidden,protected,default">;
-def fvisibility_externs_dllimport_EQ : Joined<["-"], 
"fvisibility-externs-dllimport=">, Group, Flags<[CC1Option]>,
-  HelpText<"The visibility for dllimport external declarations 
[-fvisibility-from-dllstorageclass]">, Values<"hidden,protected,default">;
-def fvisibility_externs_nodllstorageclass_EQ : Joined<["-"], 
"fvisibility-externs-nodllstorageclass=">, Group, Flags<[CC1Option]>,
-  HelpText<"The visibility for external declarations without an explicit DLL 
dllstorageclass [-fvisibility-from-dllstorageclass]">, 
Values<"hidden,protected,default">;
 def fvisibility_EQ : Joined<["-"], "fvisibility=">, Group,
   HelpText<"Set the default symbol visibility for all global declarations">, 
Values<"hidden,default">;
 def fvisibility_inlines_hidden : Flag<["-"], "fvisibility-inlines-hidden">, 
Group,

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 9512b350d9fd..66a3c57b9112 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -401,41 +401,6 @@ void InstrProfStats::reportDiagnostics(DiagnosticsEngine 
&Diags,
   }
 }
 
-static void setVisibilityFromDLLStorage

[PATCH] D82035: [PowerPC] Add Sema checks for MMA types

[Amy Kwan via Phabricator via cfe-commits]

amyk added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:3312
+// if the type is invalid.
+bool Sema::CheckPPCValidType(QualType Type, SourceLocation TypeLoc) {
+  if (Type->isPointerType() || Type->isArrayType())

Would  `CheckPPCMMAType` be a better name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82035

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

[PATCH] D81748: [PowerPC] Add MMA builtin decoding and definitions

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision as: amyk.
amyk added a comment.
This revision is now accepted and ready to land.

Thanks Baptiste. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81748

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

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: NoQ.
aaron.ballman added a subscriber: NoQ.
aaron.ballman added a comment.
Herald added a subscriber: Charusso.

In D83717#2366598 , @gamesh411 wrote:

>> Just to make sure we're on the same page -- the current approach is not 
>> flow-sensitive, and so my concern is that it won't report any true positives 
>> (not that it will be prone to false positives).
>
> Sorry about that. You are absolutely right; what I was trying to say is 
> CallGraph-based.
>
> Just some thoughts on this example:
>
> In D83717#2279263 , @aaron.ballman 
> wrote:
>
>> One of the concerns I have with this not being a flow-sensitive check is 
>> that most of the bad situations are not going to be caught by the clang-tidy 
>> version of the check. The CERT rules show contrived code examples, but the 
>> more frequent issue looks like:
>>
>>   void cleanup(struct whatever *ptr) {
>> assert(ptr); // This potentially calls abort()
>> free(ptr->buffer);
>> free(ptr);
>>   }
>>   ...
>
> What I have in support of this approach is this reasoning:
> If a handler is used where either branch can abort then that branch is 
> expected to be taken. Otherwise it is dead code. I would argue then, that 
> this abortion should be refactored out of the handler function to ensure 
> well-defined behaviour in every possible case.

If the assert was directly within the handler code, then sure. However, 
consider a situation like this:

  struct Something {
Something(int *ip) { assert(ip); ... }
...
  };

where the use of the assertion is far removed from the fact that it's being 
used within a handler.

> As a counter-argument; suppose that there is a function that is used as both 
> an exit-handler and as a simple invocation. In this case, I can understand if 
> one would not want to factor the abortion logic out, or possibly pass flags 
> around.

Yes, this is exactly the situation I'm worried about.

> Then to this remark:
>
>> The fact that we're not looking through the call sites (even without 
>> cross-TU support) means the check isn't going to catch the most problematic 
>> cases. You could modify the called function collector to gather this a bit 
>> better, but you'd issue false positives in flow-sensitive situations like:
>>
>>   void some_cleanup_func(void) {
>> for (size_t idx = 0; idx < GlobalElementCount; ++idx) {
>>   struct whatever *ptr = GlobalElement[idx];
>>   if (ptr) {
>> // Now we know abort() won't be called
>> cleanup(ptr);
>>   }
>> }
>>   }
>
> The current approach definitely does not take 'adjacent' call-sites into 
> account (not to mention CTU ones).
> In this regard I also tend to see the benefit of this being a ClangSA checker 
> as that would solve 3 problems at once:
>
> 1. Being path-sensitive, so we can explain how we got to the erroneous 
> program-point
> 2. It utilizes CTU mode to take callsites from other TU-s into account
> 3. Runtime-stack building is implicitly done by ExprEngine as a side effect 
> of symbolic execution

Agreed.

> Counter-argument:
> But using ClangSA also introduces a big challenge.
> ClangSA analyzes all top-level functions during analysis. However I don't 
> know if it understands the concept of exit-handlers, and I don't know a way 
> of 'triggering' an analysis 'on-exit' so to speak.
> So AFAIK this model of analyzing only top-level functions is a limitation 
> when it comes to modelling the program behaviour 'on-exit'.

I'm hoping someone more well-versed in the details of the static analyzer can 
speak to this point. @NoQ @Szelethus others?

> sidenote:
> To validate this claim I have dumped the exploded graph of the following file:
>
>   #include 
>   #include 
>   
>   void f() {
> std::cout << "handler f";
>   };
>   
>   int main() {
> std::atexit(f);
>   }
>
> And it has no mention of std::cout being used, so I concluded, that ClangSA 
> does not model the 'on-exit' behaviour.
>
> I wanted to clear these issues before I made the documentation.
> Thanks for the effort and the tips on evaluating the solution, I will do some 
> more exploration.

Thank you for looking into this! If it turns out that there's some reason why 
the static analyzer cannot be at least as good of a home for the functionality 
as clang-tidy, that would be really interesting to learn. Either there are 
improvements we could consider making to the static analyzer, or we could leave 
the check in clang-tidy despite the limitations, but there's still a path 
forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717

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

[clang] 2e2be3d - Correct the nomerge attribute documentation

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2020-11-02T12:49:40-05:00
New Revision: 2e2be3d964fb8f5a70faff1edbca4598bbb6ea94

URL: 
https://github.com/llvm/llvm-project/commit/2e2be3d964fb8f5a70faff1edbca4598bbb6ea94
DIFF: 
https://github.com/llvm/llvm-project/commit/2e2be3d964fb8f5a70faff1edbca4598bbb6ea94.diff

LOG: Correct the nomerge attribute documentation

The nomerge attribute is a statement attribute not a function attribute.

Added: 


Modified: 
clang/include/clang/Basic/AttrDocs.td

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 238b0752a1a2..a2f7b7a5bc1f 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -377,7 +377,7 @@ that appears to be capable of returning to its caller.
 }
 
 def NoMergeDocs : Documentation {
-  let Category = DocCatFunction;
+  let Category = DocCatStmt;
   let Content = [{
 If a statement is marked ``nomerge`` and contains call expressions, those call
 expressions inside the statement will not be merged during optimization. This 



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

[PATCH] D90246: [clang-format] Improve BAS_DontAlign+AllowAllArgumentsOnNextLine=false

[Alexander Richardson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG906b9dbc9d74: [clang-format] Improve 
BAS_DontAlign+AllowAllArgumentsOnNextLine=false (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90246

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4992,6 +4992,60 @@
Style);
 }
 
+TEST_F(FormatTest, AllowAllArgumentsOnNextLineDontAlign) {
+  // Check that AllowAllArgumentsOnNextLine is respected for both BAS_DontAlign
+  // and BAS_Align.
+  auto Style = getLLVMStyle();
+  Style.ColumnLimit = 35;
+  StringRef Input = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int A, int B, int C);";
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n"
+  "paramC);\n"
+  "void functionDecl(int A, int B,\n"
+  "int C);"),
+format(Input, Style));
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_Align;
+  EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n"
+  " paramC);\n"
+  "void functionDecl(int A, int B,\n"
+  "  int C);"),
+format(Input, Style));
+  // However, BAS_AlwaysBreak should take precedence over
+  // AllowAllArgumentsOnNextLine.
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  EXPECT_EQ(StringRef("functionCall(\n"
+  "paramA, paramB, paramC);\n"
+  "void functionDecl(\n"
+  "int A, int B, int C);"),
+format(Input, Style));
+
+  // When AllowAllArgumentsOnNextLine is set, we prefer breaking before the
+  // first argument.
+  Style.AllowAllArgumentsOnNextLine = true;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  EXPECT_EQ(StringRef("functionCall(\n"
+  "paramA, paramB, paramC);\n"
+  "void functionDecl(\n"
+  "int A, int B, int C);"),
+format(Input, Style));
+  // It wouldn't fit on one line with aligned parameters so this setting
+  // doesn't change anything for BAS_Align.
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_Align;
+  EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n"
+  " paramC);\n"
+  "void functionDecl(int A, int B,\n"
+  "  int C);"),
+format(Input, Style));
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  EXPECT_EQ(StringRef("functionCall(\n"
+  "paramA, paramB, paramC);\n"
+  "void functionDecl(\n"
+  "int A, int B, int C);"),
+format(Input, Style));
+}
+
 TEST_F(FormatTest, BreakConstructorInitializersAfterColon) {
   FormatStyle Style = getLLVMStyle();
   Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2734,7 +2734,11 @@
   if (Left.is(TT_TemplateOpener))
 return 100;
   if (Left.opensScope()) {
-if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
+// If we aren't aligning after opening parens/braces we can always break
+// here unless the style does not want us to place all arguments on the
+// next line.
+if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign &&
+(Left.ParameterCount <= 1 || Style.AllowAllArgumentsOnNextLine))
   return 0;
 if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
   return 19;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4992,6 +4992,60 @@
Style);
 }
 
+TEST_F(FormatTest, AllowAllArgumentsOnNextLineDontAlign) {
+  // Check that AllowAllArgumentsOnNextLine is respected for both BAS_DontAlign
+  // and BAS_Align.
+  auto Style = getLLVMStyle();
+  Style.ColumnLimit = 35;
+  StringRef Input = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int A, int B, int C);";
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n"

[clang] 906b9db - [clang-format] Improve BAS_DontAlign+AllowAllArgumentsOnNextLine=false

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-11-02T17:52:37Z
New Revision: 906b9dbc9d7487ee923b6a516c36777a2be0ca35

URL: 
https://github.com/llvm/llvm-project/commit/906b9dbc9d7487ee923b6a516c36777a2be0ca35
DIFF: 
https://github.com/llvm/llvm-project/commit/906b9dbc9d7487ee923b6a516c36777a2be0ca35.diff

LOG: [clang-format] Improve BAS_DontAlign+AllowAllArgumentsOnNextLine=false

TokenAnnotator::splitPenalty() was always returning 0 for opening parens if
AlignAfterOpenBracket was set to BAS_DontAlign, so the preferred point for
line breaking was always after the open paren (and was ignoring
PenaltyBreakBeforeFirstCallParameter). This change restricts the zero
penalty to the AllowAllArgumentsOnNextLine case. This results in improved
formatting for FreeBSD where we set AllowAllArgumentsOnNextLine: false
and a high value for PenaltyBreakBeforeFirstCallParameter to avoid breaking
after the open paren.

Before:
```
functionCall(
paramA, paramB, paramC);
void functionDecl(
int A, int B, int C)
```
After:
```
functionCall(paramA, paramB,
paramC);
void functionDecl(int A, int B,
int C)
```

Reviewed By: MyDeveloperDay

Differential Revision: https://reviews.llvm.org/D90246

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 22dcd7521dbe..0fdcca867e3d 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2734,7 +2734,11 @@ unsigned TokenAnnotator::splitPenalty(const 
AnnotatedLine &Line,
   if (Left.is(TT_TemplateOpener))
 return 100;
   if (Left.opensScope()) {
-if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign)
+// If we aren't aligning after opening parens/braces we can always break
+// here unless the style does not want us to place all arguments on the
+// next line.
+if (Style.AlignAfterOpenBracket == FormatStyle::BAS_DontAlign &&
+(Left.ParameterCount <= 1 || Style.AllowAllArgumentsOnNextLine))
   return 0;
 if (Left.is(tok::l_brace) && !Style.Cpp11BracedListStyle)
   return 19;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 0c08eb55be5f..3d4e3e445c78 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -4992,6 +4992,60 @@ TEST_F(FormatTest, AllowAllArgumentsOnNextLine) {
Style);
 }
 
+TEST_F(FormatTest, AllowAllArgumentsOnNextLineDontAlign) {
+  // Check that AllowAllArgumentsOnNextLine is respected for both BAS_DontAlign
+  // and BAS_Align.
+  auto Style = getLLVMStyle();
+  Style.ColumnLimit = 35;
+  StringRef Input = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int A, int B, int C);";
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n"
+  "paramC);\n"
+  "void functionDecl(int A, int B,\n"
+  "int C);"),
+format(Input, Style));
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_Align;
+  EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n"
+  " paramC);\n"
+  "void functionDecl(int A, int B,\n"
+  "  int C);"),
+format(Input, Style));
+  // However, BAS_AlwaysBreak should take precedence over
+  // AllowAllArgumentsOnNextLine.
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  EXPECT_EQ(StringRef("functionCall(\n"
+  "paramA, paramB, paramC);\n"
+  "void functionDecl(\n"
+  "int A, int B, int C);"),
+format(Input, Style));
+
+  // When AllowAllArgumentsOnNextLine is set, we prefer breaking before the
+  // first argument.
+  Style.AllowAllArgumentsOnNextLine = true;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  EXPECT_EQ(StringRef("functionCall(\n"
+  "paramA, paramB, paramC);\n"
+  "void functionDecl(\n"
+  "int A, int B, int C);"),
+format(Input, Style));
+  // It wouldn't fit on one line with aligned parameters so this setting
+  // doesn't change anything for BAS_Align.
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_Align;
+  EXPECT_EQ(StringRef("functionCall(paramA, paramB,\n"
+  " paramC);\n"
+  "void functionDecl(int A, int B,\n"
+  "  int C);"),
+format(Input, Style));
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  EXPECT_EQ(StringRef("functionCall(\n"
+  "paramA, paramB, paramC);\n"

[PATCH] D90507: Adding DWARF64 clang flag

[Alexander Yermolovich via Phabricator via cfe-commits]

ayermolo updated this revision to Diff 302326.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -905,6 +905,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = !Args.hasArg(OPT_fno_verbose_asm);
+  Opts.Dwarf64 = Args.hasArg(OPT_gdwarf64);
   Opts.PreserveAsmComments = !Args.hasArg(OPT_fno_preserve_as_comments);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4760,6 +4760,9 @@
 IsIntegratedAssemblerDefault))
 CmdArgs.push_back("-fno-verbose-asm");
 
+  if(Args.hasArg(options::OPT_gdwarf64))
+CmdArgs.push_back("-gdwarf64");
+
   if (!TC.useIntegratedAs())
 CmdArgs.push_back("-no-integrated-as");
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -561,6 +561,7 @@
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptions.MCNoWarn = CodeGenOpts.NoWarn;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
+  Options.MCOptions.Dwarf64 = CodeGenOpts.Dwarf64;
   Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
   Options.MCOptions.ABIName = TargetOpts.ABI;
   for (const auto &Entry : HSOpts.UserEntries)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2084,6 +2084,8 @@
   HelpText<"Generate source-level debug information with dwarf version 4">;
 def gdwarf_5 : Flag<["-"], "gdwarf-5">, Group,
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 
 def gcodeview : Flag<["-"], "gcodeview">,
   HelpText<"Generate CodeView debug information">,
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -32,6 +32,7 @@
 llvm::DebugCompressionType::None)
 CODEGENOPT(RelaxELFRelocations, 1, 0) ///< -Wa,--mrelax-relocations
 CODEGENOPT(AsmVerbose, 1, 0) ///< -dA, -fverbose-asm.
+CODEGENOPT(Dwarf64   , 1, 0) ///< -gdwarf64.
 CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.
 CODEGENOPT(AssumeSaneOperatorNew , 1, 1) ///< implicit __attribute__((malloc)) 
operator new
 CODEGENOPT(Autolink  , 1, 1) ///< -fno-autolink


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -905,6 +905,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = !Args.hasArg(OPT_fno_verbose_asm);
+  Opts.Dwarf64 = Args.hasArg(OPT_gdwarf64);
   Opts.PreserveAsmComments = !Args.hasArg(OPT_fno_preserve_as_comments);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4760,6 +4760,9 @@
 IsIntegratedAssemblerDefault))
 CmdArgs.push_back("-fno-verbose-asm");
 
+  if(Args.hasArg(options::OPT_gdwarf64))
+CmdArgs.push_back("-gdwarf64");
+
   if (!TC.useIntegratedAs())
 CmdArgs.push_back("-no-integrated-as");
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -561,6 +561,7 @@
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptio

[PATCH] D90630: [CodeGen] Fix Bug 47499: __unaligned extension inconsistent behaviour with C and C++

[Jan Ole Hüser via Phabricator via cfe-commits]

j0le created this revision.
j0le added reviewers: rogfer01, rnk.
j0le added a project: clang.
Herald added a subscriber: cfe-commits.
j0le requested review of this revision.

Hello everyone,

I think, I have found the reason, why there is a difference between C and C++ 
for the keyword __unaligned:
For C, the Method getAsCXXREcordDecl() returns nullptr. That guarantees that 
hasUnaligned() is called.
If the language is C++, it is not guaranteed, that hasUnaligend() is called and 
evaluated.

I have some questions:

- Does this CXXRecordDecl contain information, whether the keyword  __unaligned 
is used?
- Does getClassPointerAlignment() has some side effects, that are needed?

Here are some links:

https://bugs.llvm.org/show_bug.cgi?id=47499
Thread on the cfe-dev mailing list: 
http://lists.llvm.org/pipermail/cfe-dev/2020-September/066783.html
Diff, that introduced the check hasUnaligned() in getNaturalTypeAlignment(): 
https://reviews.llvm.org/D30166

Kind Regards
Ole

(Jan Ole Hüser)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90630

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/unaligned-struct-copy.c


Index: clang/test/CodeGen/unaligned-struct-copy.c
===
--- /dev/null
+++ clang/test/CodeGen/unaligned-struct-copy.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -xc   -O2 -triple thumbv7a-unknown-windows-eabi 
-fms-extensions -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc++ -O2 -triple thumbv7a-unknown-windows-eabi 
-fms-extensions -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc   -O2 -triple x86_64-unknown-linux-gnu -fms-extensions 
-emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc++ -O2 -triple x86_64-unknown-linux-gnu -fms-extensions 
-emit-llvm < %s | FileCheck %s
+
+struct S1 {
+  unsigned long x;
+};
+
+// CHECK: define
+// CHECK-SAME: void
+// CHECK-SAME: test1
+
+void test1(__unaligned struct S1 *out) {
+  // CHECK: store
+  // CHECK-SAME: align 1
+  out->x = 5;
+  // CHECK: ret void
+}
+
+// CHECK: define
+// CHECK-SAME: void
+// CHECK-SAME: test2
+
+void test2(__unaligned struct S1 *out, __unaligned struct S1 *in) {
+  // CHECK: load
+  // CHECK-SAME: align 1
+  // CHECK: store
+  // CHECK-SAME: align 1
+  *out = *in;
+  // CHECK: ret void
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6146,16 +6146,17 @@
 *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
 
   CharUnits Alignment;
+  const CXXRecordDecl *RD;
+  if (T.getQualifiers().hasUnaligned()) {
+Alignment = CharUnits::One();
+  }
   // For C++ class pointees, we don't know whether we're pointing at a
   // base or a complete object, so we generally need to use the
   // non-virtual alignment.
-  const CXXRecordDecl *RD;
-  if (forPointeeType && !AlignForArray && (RD = T->getAsCXXRecordDecl())) {
+  else if (forPointeeType && !AlignForArray && (RD = T->getAsCXXRecordDecl())) 
{
 Alignment = getClassPointerAlignment(RD);
   } else {
 Alignment = getContext().getTypeAlignInChars(T);
-if (T.getQualifiers().hasUnaligned())
-  Alignment = CharUnits::One();
   }
 
   // Cap to the global maximum type alignment unless the alignment


Index: clang/test/CodeGen/unaligned-struct-copy.c
===
--- /dev/null
+++ clang/test/CodeGen/unaligned-struct-copy.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -xc   -O2 -triple thumbv7a-unknown-windows-eabi -fms-extensions -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc++ -O2 -triple thumbv7a-unknown-windows-eabi -fms-extensions -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc   -O2 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc++ -O2 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+
+struct S1 {
+  unsigned long x;
+};
+
+// CHECK: define
+// CHECK-SAME: void
+// CHECK-SAME: test1
+
+void test1(__unaligned struct S1 *out) {
+  // CHECK: store
+  // CHECK-SAME: align 1
+  out->x = 5;
+  // CHECK: ret void
+}
+
+// CHECK: define
+// CHECK-SAME: void
+// CHECK-SAME: test2
+
+void test2(__unaligned struct S1 *out, __unaligned struct S1 *in) {
+  // CHECK: load
+  // CHECK-SAME: align 1
+  // CHECK: store
+  // CHECK-SAME: align 1
+  *out = *in;
+  // CHECK: ret void
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6146,16 +6146,17 @@
 *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
 
   CharUnits Alignment;
+  const CXXRecordDecl *RD;
+  if (T.getQualifiers().hasUnaligned()) {
+Alignment = CharUnits::One();
+  }
   // For C++ class pointees, we don't know whether we're pointing at a
   // base or a complete object, so 

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Given that some parts of this confused me, I'd like to make sure I'm clear on 
the approach you're taking. Based on the fact that labels are statements 
(defined as [stmt.label], in the statment production, etc), I would expect that 
attributes which appertain to labels all appertain to the statement and not the 
declaration and that the underlying issue is that we attach attributes to the 
label declaration (such as for `__attribute__((unused))`. So I wasn't expecting 
that we'd slide attributes around, but redefine which construct they live on 
(they'd always make an `AttributedStmt` for the label rather than adding the 
attribute to the `Decl`). However, I could imagine there being cases when we 
might want a helper function on `LabelDecl` that looks for attributes on the 
associated `LabelStmt` and returns results about it if that helps ease 
implementation or refactoring burdens. WDYT? Also, does @rsmith have opinions 
here?




Comment at: clang/include/clang/Sema/Sema.h:4528
+  StmtResult ActOnLabelStmt(SourceLocation IdentLoc,
+const ParsedAttributesView *Attrs,
+SourceRange AttrsRange, LabelDecl *TheDecl,

Don't we have a `ParsedAttributesViewWithRange` (or something along those 
lines) that could be used instead of separating attributes and their source 
range?



Comment at: clang/lib/Parse/ParseStmt.cpp:677
+  }
+  SourceRange AttributeRange = attrs.Range;
   attrs.clear();

I think I would have expected this to be calling 
`Actions.ProcessStmtAttributes()` as done a few lines up on 651 rather than 
changing the interface to `ActOnLabelStmt()`.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7368
+// The statement attributes attached to a LabelDecl are handled separately.
+if (!isa(D))
+  S.Diag(AL.getLoc(), diag::err_stmt_attribute_invalid_on_decl)

This also surprises me -- I wouldn't have expected the attribute to be in the 
list for the label decl in the first place.



Comment at: clang/lib/Sema/TreeTransform.h:1307
+// already properly rebuild. Only the LabelStmt itself needs to be rebuild.
+return SemaRef.ActOnLabelStmt(IdentLoc, nullptr, SourceLocation(), L,
+  ColonLoc, SubStmt);

I wouldn't have expected any changes to be needed here because the attributed 
statement should be rebuilt properly by `RebuildAttributedStmt()`, no?


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

https://reviews.llvm.org/D86559

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

[PATCH] D90630: [CodeGen] Fix Bug 47499: __unaligned extension inconsistent behaviour with C and C++

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Thanks, this basically looks correct to me, aside from some formatting details. 
Do you want me to apply those fixes and land this for you? I see that was done 
for your last patch, but it's best to ask first.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6153-6155
   // For C++ class pointees, we don't know whether we're pointing at a
   // base or a complete object, so we generally need to use the
   // non-virtual alignment.

It's pretty uncommon to see comment blocks between the closing if curly and the 
next else if. Please move this comment inside the relevant else-if block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90630

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

[clang] ba18bc4 - [Sema] adds -Wfree-nonheap-object member var checks

[George Burgess IV via cfe-commits]

Author: Christopher Di Bella
Date: 2020-11-02T11:03:28-08:00
New Revision: ba18bc4925d8cbd4a9354629617cbcafbbd48941

URL: 
https://github.com/llvm/llvm-project/commit/ba18bc4925d8cbd4a9354629617cbcafbbd48941
DIFF: 
https://github.com/llvm/llvm-project/commit/ba18bc4925d8cbd4a9354629617cbcafbbd48941.diff

LOG: [Sema] adds -Wfree-nonheap-object member var checks

Checks to make sure that stdlib's (std::)free is being appropriately
used for member variables.

Differential Revision: https://reviews.llvm.org/D90269

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/warn-free-nonheap-object.c
clang/test/Sema/warn-free-nonheap-object.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3d5e2d70d8ca..bad9b14c1fa2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10107,19 +10107,9 @@ void Sema::CheckStrncatArguments(const CallExpr *CE,
 }
 
 namespace {
-void CheckFreeArgumentsAddressof(Sema &S, const std::string &CalleeName,
- const UnaryOperator *UnaryExpr) {
-  if (UnaryExpr->getOpcode() != UnaryOperator::Opcode::UO_AddrOf)
-return;
-
-  const auto *Lvalue = dyn_cast(UnaryExpr->getSubExpr());
-  if (Lvalue == nullptr)
-return;
-
-  const auto *Var = dyn_cast(Lvalue->getDecl());
-  if (Var == nullptr)
-return;
-
+void CheckFreeArgumentsOnLvalue(Sema &S, const std::string &CalleeName,
+const UnaryOperator *UnaryExpr,
+const VarDecl *Var) {
   StorageClass Class = Var->getStorageClass();
   if (Class == StorageClass::SC_Extern ||
   Class == StorageClass::SC_PrivateExtern ||
@@ -10130,6 +10120,27 @@ void CheckFreeArgumentsAddressof(Sema &S, const 
std::string &CalleeName,
   << CalleeName << Var;
 }
 
+void CheckFreeArgumentsOnLvalue(Sema &S, const std::string &CalleeName,
+const UnaryOperator *UnaryExpr, const Decl *D) 
{
+  if (const auto *Field = dyn_cast(D))
+S.Diag(UnaryExpr->getBeginLoc(), diag::warn_free_nonheap_object)
+<< CalleeName << Field;
+}
+
+void CheckFreeArgumentsAddressof(Sema &S, const std::string &CalleeName,
+ const UnaryOperator *UnaryExpr) {
+  if (UnaryExpr->getOpcode() != UnaryOperator::Opcode::UO_AddrOf)
+return;
+
+  if (const auto *Lvalue = dyn_cast(UnaryExpr->getSubExpr()))
+if (const auto *Var = dyn_cast(Lvalue->getDecl()))
+  return CheckFreeArgumentsOnLvalue(S, CalleeName, UnaryExpr, Var);
+
+  if (const auto *Lvalue = dyn_cast(UnaryExpr->getSubExpr()))
+return CheckFreeArgumentsOnLvalue(S, CalleeName, UnaryExpr,
+  Lvalue->getMemberDecl());
+}
+
 void CheckFreeArgumentsStackArray(Sema &S, const std::string &CalleeName,
   const DeclRefExpr *Lvalue) {
   if (!Lvalue->getType()->isArrayType())

diff  --git a/clang/test/Sema/warn-free-nonheap-object.c 
b/clang/test/Sema/warn-free-nonheap-object.c
index e149e8349571..1618a559b431 100644
--- a/clang/test/Sema/warn-free-nonheap-object.c
+++ b/clang/test/Sema/warn-free-nonheap-object.c
@@ -4,6 +4,11 @@ typedef __SIZE_TYPE__ size_t;
 void *malloc(size_t);
 void free(void *);
 
+struct S {
+  int I;
+  char *P;
+};
+
 int GI;
 void test() {
   {
@@ -31,4 +36,9 @@ void test() {
 free(A);  // expected-warning {{attempt to call free on non-heap object 
'A'}}
 free(&A); // expected-warning {{attempt to call free on non-heap object 
'A'}}
   }
+  {
+struct S s;
+free(&s.I); // expected-warning {{attempt to call free on non-heap object 
'I'}}
+free(s.P);
+  }
 }

diff  --git a/clang/test/Sema/warn-free-nonheap-object.cpp 
b/clang/test/Sema/warn-free-nonheap-object.cpp
index 0578d9e9cd66..9347709a23ca 100644
--- a/clang/test/Sema/warn-free-nonheap-object.cpp
+++ b/clang/test/Sema/warn-free-nonheap-object.cpp
@@ -13,10 +13,25 @@ int GI;
 struct S {
   operator char *() { return ptr; }
 
+  void CFree() {
+::free(&ptr); // expected-warning {{attempt to call free on non-heap 
object 'ptr'}}
+::free(&I);   // expected-warning {{attempt to call free on non-heap 
object 'I'}}
+::free(ptr);
+  }
+
+  void CXXFree() {
+std::free(&ptr); // expected-warning {{attempt to call std::free on 
non-heap object 'ptr'}}
+std::free(&I);   // expected-warning {{attempt to call std::free on 
non-heap object 'I'}}
+std::free(ptr);
+  }
+
 private:
   char *ptr = (char *)std::malloc(10);
+  static int I;
 };
 
+int S::I = 0;
+
 void test1() {
   {
 free(&GI); // expected-warning {{attempt to call free on non-heap object 
'GI'}}
@@ -51,6 +66,10 @@ void test1() {
 free(s);
 free(&s); // expected-warning {{attempt to call free on non-heap object 
's'}}
   }
+  {
+S s;
+s.CFree();
+  }
 }
 
 void test2() {
@@ -87,4 +106,8 @@ void test2(

[PATCH] D90572: [clang] [MinGW] Allow using the vptr sanitizer

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90572

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

[PATCH] D90269: adds -Wfree-nonheap-object member var checks

[George Burgess IV via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba18bc4925d8: [Sema] adds -Wfree-nonheap-object member var 
checks (authored by cjdb, committed by george.burgess.iv).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90269

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/warn-free-nonheap-object.c
  clang/test/Sema/warn-free-nonheap-object.cpp

Index: clang/test/Sema/warn-free-nonheap-object.cpp
===
--- clang/test/Sema/warn-free-nonheap-object.cpp
+++ clang/test/Sema/warn-free-nonheap-object.cpp
@@ -13,10 +13,25 @@
 struct S {
   operator char *() { return ptr; }
 
+  void CFree() {
+::free(&ptr); // expected-warning {{attempt to call free on non-heap object 'ptr'}}
+::free(&I);   // expected-warning {{attempt to call free on non-heap object 'I'}}
+::free(ptr);
+  }
+
+  void CXXFree() {
+std::free(&ptr); // expected-warning {{attempt to call std::free on non-heap object 'ptr'}}
+std::free(&I);   // expected-warning {{attempt to call std::free on non-heap object 'I'}}
+std::free(ptr);
+  }
+
 private:
   char *ptr = (char *)std::malloc(10);
+  static int I;
 };
 
+int S::I = 0;
+
 void test1() {
   {
 free(&GI); // expected-warning {{attempt to call free on non-heap object 'GI'}}
@@ -51,6 +66,10 @@
 free(s);
 free(&s); // expected-warning {{attempt to call free on non-heap object 's'}}
   }
+  {
+S s;
+s.CFree();
+  }
 }
 
 void test2() {
@@ -87,4 +106,8 @@
 std::free(s);
 std::free(&s); // expected-warning {{attempt to call std::free on non-heap object 's'}}
   }
+  {
+S s;
+s.CXXFree();
+  }
 }
Index: clang/test/Sema/warn-free-nonheap-object.c
===
--- clang/test/Sema/warn-free-nonheap-object.c
+++ clang/test/Sema/warn-free-nonheap-object.c
@@ -4,6 +4,11 @@
 void *malloc(size_t);
 void free(void *);
 
+struct S {
+  int I;
+  char *P;
+};
+
 int GI;
 void test() {
   {
@@ -31,4 +36,9 @@
 free(A);  // expected-warning {{attempt to call free on non-heap object 'A'}}
 free(&A); // expected-warning {{attempt to call free on non-heap object 'A'}}
   }
+  {
+struct S s;
+free(&s.I); // expected-warning {{attempt to call free on non-heap object 'I'}}
+free(s.P);
+  }
 }
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10107,19 +10107,9 @@
 }
 
 namespace {
-void CheckFreeArgumentsAddressof(Sema &S, const std::string &CalleeName,
- const UnaryOperator *UnaryExpr) {
-  if (UnaryExpr->getOpcode() != UnaryOperator::Opcode::UO_AddrOf)
-return;
-
-  const auto *Lvalue = dyn_cast(UnaryExpr->getSubExpr());
-  if (Lvalue == nullptr)
-return;
-
-  const auto *Var = dyn_cast(Lvalue->getDecl());
-  if (Var == nullptr)
-return;
-
+void CheckFreeArgumentsOnLvalue(Sema &S, const std::string &CalleeName,
+const UnaryOperator *UnaryExpr,
+const VarDecl *Var) {
   StorageClass Class = Var->getStorageClass();
   if (Class == StorageClass::SC_Extern ||
   Class == StorageClass::SC_PrivateExtern ||
@@ -10130,6 +10120,27 @@
   << CalleeName << Var;
 }
 
+void CheckFreeArgumentsOnLvalue(Sema &S, const std::string &CalleeName,
+const UnaryOperator *UnaryExpr, const Decl *D) {
+  if (const auto *Field = dyn_cast(D))
+S.Diag(UnaryExpr->getBeginLoc(), diag::warn_free_nonheap_object)
+<< CalleeName << Field;
+}
+
+void CheckFreeArgumentsAddressof(Sema &S, const std::string &CalleeName,
+ const UnaryOperator *UnaryExpr) {
+  if (UnaryExpr->getOpcode() != UnaryOperator::Opcode::UO_AddrOf)
+return;
+
+  if (const auto *Lvalue = dyn_cast(UnaryExpr->getSubExpr()))
+if (const auto *Var = dyn_cast(Lvalue->getDecl()))
+  return CheckFreeArgumentsOnLvalue(S, CalleeName, UnaryExpr, Var);
+
+  if (const auto *Lvalue = dyn_cast(UnaryExpr->getSubExpr()))
+return CheckFreeArgumentsOnLvalue(S, CalleeName, UnaryExpr,
+  Lvalue->getMemberDecl());
+}
+
 void CheckFreeArgumentsStackArray(Sema &S, const std::string &CalleeName,
   const DeclRefExpr *Lvalue) {
   if (!Lvalue->getType()->isArrayType())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90362: scan-build supprot relative 'file' in cdb.

[Yu Shan via Phabricator via cfe-commits]

aabbaabb added a comment.

kindly ping for help to submit this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90362

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem updated this revision to Diff 302349.
gulfem marked 3 inline comments as done.
gulfem added a comment.

Addressed the comments about the documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-leaf.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-leaf.c
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll

Index: llvm/test/Bitcode/attributes.ll
===
--- llvm/test/Bitcode/attributes.ll
+++ llvm/test/Bitcode/attributes.ll
@@ -410,6 +410,12 @@
   ret void
 }
 
+; CHECK; define void @f70() #43
+define void @f70() nocallback
+{
+  ret void
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
@@ -453,4 +459,5 @@
 ; CHECK: attributes #40 = { null_pointer_is_valid }
 ; CHECK: attributes #41 = { mustprogress }
 ; CHECK: attributes #42 = { nossp }
+; CHECK: attributes #43 = { nocallback }
 ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin }
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -906,6 +906,7 @@
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
   case Attribute::MinSize:
+  case Attribute::NoCallback:
   case Attribute::NoDuplicate:
   case Attribute::NoFree:
   case Attribute::NoImplicitFloat:
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1572,6 +1572,7 @@
   case Attribute::NoReturn:
   case Attribute::NoSync:
   case Attribute::WillReturn:
+  case Attribute::NoCallback:
   case Attribute::NoCfCheck:
   case Attribute::NoUnwind:
   case Attribute::NoInline:
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -371,6 +371,8 @@
 return "noalias";
   if (hasAttribute(Attribute::NoBuiltin))
 return "nobuiltin";
+  if (hasAttribute(Attribute::NoCallback))
+return "nocallback";
   if (hasAttribute(Attribute::NoCapture))
 return "nocapture";
   if (hasAttribute(Attribute::NoDuplicate))
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -643,6 +643,8 @@
 return bitc::ATTR_KIND_NO_ALIAS;
   case Attribute::NoBuiltin:
 return bitc::ATTR_KIND_NO_BUILTIN;
+  case Attribute::NoCallback:
+return bitc::ATTR_KIND_NO_CALLBACK;
   case Attribute::NoCapture:
 return bitc::ATTR_KIND_NO_CAPTURE;
   case Attribute::NoDuplicate:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1433,6 +1433,8 @@
 return Attribute::NoAlias;
   case bitc::ATTR_KIND_NO_BUILTIN:
 return Attribute::NoBuiltin;
+  case bitc::ATTR_KIND_NO_CALLBACK:
+return Attribute::NoCallback;
   case bitc::ATTR_KIND_NO_CAPTURE:
 return Attribute::NoCapture;
   case bitc::ATTR_KIND_NO_DUPLICATE:
Index: llvm/lib/AsmParser/LLToken.h
===
--- llvm/lib/AsmParser/LLToken.h
+++ llvm/lib/AsmParser/LLToken.h
@@ -198,6 +198,7 @@
   kw_noalias,
   kw_noundef,
   kw_nobuiltin,
+  kw_nocallback,
   kw_nocapture,
   kw_noduplicate,
   kw_nofree,
Index: llvm/lib/AsmParser/LLParser.cpp
===
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1314,6 +1314,9 @@
   break;
 case lltok::kw_naked: B.addAttribute(Attribute::Naked); break;
 case lltok::kw_nobuiltin: B.addAttribute(Attribute::NoBuiltin); break;
+case lltok::kw_nocallback:
+  B.addAttribute(Attribute::NoCallback);
+  break;
 case lltok::kw_noduplicate: B.addAttribute(Attribute::NoDuplicate); break;
 case lltok::kw_nofree: B.addAttribute(Attribute::NoFree); break;
 case lltok::kw_noimplicitfloat:
Index: llvm/lib/AsmParser/LLLexer.cpp

[PATCH] D90634: Implement Lambda Conversion Operators for All CCs for MSVC.

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: rjmccall, aaron.ballman.
Herald added subscribers: mgrang, kristof.beyls.
erichkeane requested review of this revision.

As described here:
https://devblogs.microsoft.com/oldnewthing/20150220-00/?p=44623

In order to allow Lambdas to be used with traditional Win32 APIs, they
emit a conversion function for (what Raymond Chen claims is all) a
number of the calling conventions.  Through experimentation, we
discovered that the list isn't quite 'all'.

This patch implements this by taking the list of conversions that MSVC
emits (across 'all' architectures, I don't see any CCs on ARM), then
emits them if they are supported by the current target.

However, we also add 3 other options (which may be duplicates):
free-function, member-function, and operator() calling conventions.  We
do this because we have an extension where we generate both free and
member for these cases so th at people specifying a calling convention
on the lambda will have the expected behavior when specifying one of
those two.

MSVC doesn't seem to permit specifying calling-convention on lambdas,
but we do, so we need to make sure those are emitted as well. We do this
so that clang-only conventions are supported if the user specifies them.


https://reviews.llvm.org/D90634

Files:
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCXX/lambda-conversion-op-cc.cpp

Index: clang/test/CodeGenCXX/lambda-conversion-op-cc.cpp
===
--- clang/test/CodeGenCXX/lambda-conversion-op-cc.cpp
+++ clang/test/CodeGenCXX/lambda-conversion-op-cc.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,LIN64
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-linux-gnu -DCC="__attribute__((vectorcall))" | FileCheck %s --check-prefixes=CHECK,VECCALL
-// RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-windows-pc -DWIN32 | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -emit-llvm %s -o - -fms-compatibility -triple=i386-windows-pc -DWIN32 | FileCheck %s --check-prefixes=WIN32
 
 #ifndef CC
 #define CC
@@ -10,20 +10,31 @@
   auto lambda = [](int i, float f, double d) CC { return i + f + d; };
 
   double (*CC fp)(int, float, double) = lambda;
-  fp(0, 1.1, 2.2);
 #ifdef WIN32
   double (*__attribute__((thiscall)) fp2)(int, float, double) = lambda;
+  double (*__attribute__((stdcall)) fp3)(int, float, double) = lambda;
+  double (*__attribute__((fastcall)) fp4)(int, float, double) = lambda;
+  double (*__attribute__((vectorcall)) fp5)(int, float, double) = lambda;
+#endif // WIN32
+  fp(0, 1.1, 2.2);
+#ifdef WIN32
   fp2(0, 1.1, 2.2);
+  fp3(0, 1.1, 2.2);
+  fp4(0, 1.1, 2.2);
+  fp5(0, 1.1, 2.2);
 #endif // WIN32
 }
 
-// void usage function, calls convrsion operator.
+// void usage function, calls conversion operator.
 // LIN64: define void @_Z5usagev()
 // VECCALL: define void @_Z5usagev()
 // WIN32: define dso_local void @"?usage@@YAXXZ"()
 // CHECK: call double (i32, float, double)* @"_ZZ5usagevENK3$_0cvPFdifdEEv"
 // WIN32: call x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6A?A?@@HMN@ZXZ"
 // WIN32: call x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6E?A?@@HMN@ZXZ"
+// WIN32: call x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6G?A?@@HMN@ZXZ"
+// WIN32: call x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6I?A?@@HMN@ZXZ"
+// WIN32: call x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6Q?A?@@HMN@ZXZ"
 //
 // Conversion operator, returns __invoke.
 // CHECK: define internal double (i32, float, double)* @"_ZZ5usagevENK3$_0cvPFdifdEEv"
@@ -32,6 +43,12 @@
 // WIN32: ret double (i32, float, double)* @"?__invoke@@?0??usage@@YAXXZ@CA?A?@@HMN@Z"
 // WIN32: define internal x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6E?A?@@HMN@ZXZ"
 // WIN32: ret double (i32, float, double)* @"?__invoke@@?0??usage@@YAXXZ@CE?A?@@HMN@Z"
+// WIN32: define internal x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6G?A?@@HMN@ZXZ"
+// WIN32: ret double (i32, float, double)* @"?__invoke@@?0??usage@@YAXXZ@CG?A?@@HMN@Z"
+// WIN32: define internal x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6I?A?@@HMN@ZXZ"
+// WIN32: ret double (i32, float, double)* @"?__invoke@@?0??usage@@YAXXZ@CI?A?@@HMN@Z"
+// WIN32: define internal x86_thiscallcc double (i32, float, double)* @"??B@?0??usage@@YAXXZ@QBEP6Q?A?@@HMN@ZXZ"
+// WIN32: ret double (i32, float, double)* @"?__invoke@@?0??usage@@YAXXZ@CQ?A?@@HMN@Z"
 //
 // __invoke function, calls operator(). Win32 should call both.
 // LIN64: define internal double @"_ZZ5usagevEN3$_08__invokeEifd"
@@ -42,3 +59,9 @@
 // WIN32: call x86_thiscallcc double @"??R@?0??usage@@YAXXZ@QBE?A?@@HMN@Z"
 // WIN32: define internal x86_thiscallcc double @"?__invoke@@?0??usage@@YAXXZ@CE?A?@@HMN@Z"
 /

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem marked an inline comment as not done.
gulfem added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3909
+The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis 
in library functions.
+Functions marked as ``leaf`` attribute are not allowed to enter their caller's 
translation unit.
+Therefore, they cannot use or modify any data that does not escape the current 
compilation unit.

aaron.ballman wrote:
> as leaf -> with the leaf
> 
> I'm not certain how to interpret that functions are not allowed to enter 
> their caller's translation unit. I sort of read that as leaf functions are 
> not allowed to call (or otherwise jump) out of the translation unit in which 
> they're defined -- is that about right?
I think the description is a little confusing.
As far as I understand, leaf functions can actually call or jump out the the 
translation unit that they are defined ("Leaf functions might still call 
functions from other compilation units").
The manual refers caller function's translation unit as **current translation** 
unit.
"Calls to external functions with this attribute must return to the current 
compilation unit only by return or by exception handling. In particular, a leaf 
function is not allowed to invoke callback functions passed to it from the 
current compilation unit, directly call functions exported by the unit, or 
longjmp into the unit."
My interpretation of this statement is that a function marked with a leaf 
attribute can only return to its caller translation unit by a return or an 
exception, but it cannot enter into callers translation unit by invoking a 
callback function. 
Does that make sense?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3909
+The ``leaf`` attribute is used as a compiler hint to improve dataflow analysis 
in library functions.
+Functions marked as ``leaf`` attribute are not allowed to enter their caller's 
translation unit.
+Therefore, they cannot use or modify any data that does not escape the current 
compilation unit.

gulfem wrote:
> aaron.ballman wrote:
> > as leaf -> with the leaf
> > 
> > I'm not certain how to interpret that functions are not allowed to enter 
> > their caller's translation unit. I sort of read that as leaf functions are 
> > not allowed to call (or otherwise jump) out of the translation unit in 
> > which they're defined -- is that about right?
> I think the description is a little confusing.
> As far as I understand, leaf functions can actually call or jump out the the 
> translation unit that they are defined ("Leaf functions might still call 
> functions from other compilation units").
> The manual refers caller function's translation unit as **current 
> translation** unit.
> "Calls to external functions with this attribute must return to the current 
> compilation unit only by return or by exception handling. In particular, a 
> leaf function is not allowed to invoke callback functions passed to it from 
> the current compilation unit, directly call functions exported by the unit, 
> or longjmp into the unit."
> My interpretation of this statement is that a function marked with a leaf 
> attribute can only return to its caller translation unit by a return or an 
> exception, but it cannot enter into callers translation unit by invoking a 
> callback function. 
> Does that make sense?
> 
> 
Ah, thank you for the explanation! How about: `Functions marked with the 
''leaf'' attribute are not allowed to jump back into the caller's translation 
unit, whether through invoking a callback function, a direct external function 
call, use of 'longjmp', or other means.`?

Is this property transitive? e.g.,
```
// TU1.c
void func(void) {
  leaf();
}

void bar(void) {}

// TU2.c
__attribute__((leaf)) void leaf(void) {
  baz(); // Is this not allowed?
}

// TU3.c
void baz(void) {
  bar();
}
```
The "directly call functions exported by the unit" makes me think the above 
code is fine but it could also be that "direct" in this case means "through a 
function designator rather than a function pointer".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1435
+  let Spellings = [GCC<"leaf">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

aaron.ballman wrote:
> Should this attribute also be supported on things like ObjC method decls or 
> other function-like interfaces?
Do I need to do anything else to support this attribute in Objective-C as well?
I think we should support it in all the C languages family.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D90275: [clang][IR] Add support for leaf attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1435
+  let Spellings = [GCC<"leaf">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

gulfem wrote:
> aaron.ballman wrote:
> > Should this attribute also be supported on things like ObjC method decls or 
> > other function-like interfaces?
> Do I need to do anything else to support this attribute in Objective-C as 
> well?
> I think we should support it in all the C languages family.
>I think we should support it in all the C languages family.

That's already happening automatically -- there's a C and C++ spelling 
available for it and the attribute doesn't specify that it requires a 
particular language mode or target.

> Do I need to do anything else to support this attribute in Objective-C as 
> well?
You can add multiple subjects to the list here, so you can have this apply to 
`Function, ObjCMethod` for both of those. Another one to consider is whether 
this attribute can be written on a block declaration (like a lambda, but with 
different syntax). Beyond that, it's mostly just documentation, devising the 
test cases to ensure the ObjC functionality behaves as expected, possibly some 
codegen changes, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90275

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

[PATCH] D78899: [Driver] Add callback to Command execution

[Alexandre Ganea via Phabricator via cfe-commits]

aganea accepted this revision.
aganea added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78899

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

[PATCH] D90436: [Bundler] Use argv[0] as the default choice for the Executable name.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90436

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

[PATCH] D78899: [Driver] Add callback to Command execution

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Compilation.h:118
 
+  /// Callback called after the command has been executed.
+  std::function PostCallback;

Can you comment what `int` means?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78899

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

[PATCH] D90526: [clangd] Add -log=public to redact all request info from index server logs

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 302360.
sammccall added a comment.

Use separate flag for log-public vs log level


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90526

Files:
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/support/Logger.cpp
  clang-tools-extra/clangd/support/Logger.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -67,7 +67,8 @@
 
 private:
   // Color logs so we can distinguish them from test output.
-  void log(Level L, const llvm::formatv_object_base &Message) override {
+  void log(Level L, const char *Fmt,
+   const llvm::formatv_object_base &Message) override {
 raw_ostream::Colors Color;
 switch (L) {
 case Level::Verbose:
Index: clang-tools-extra/clangd/support/Logger.h
===
--- clang-tools-extra/clangd/support/Logger.h
+++ clang-tools-extra/clangd/support/Logger.h
@@ -24,16 +24,19 @@
 public:
   virtual ~Logger() = default;
 
-  enum Level { Debug, Verbose, Info, Error };
+  /// The significance or severity of this message.
+  /// Typically used to filter the output to an interesting level.
+  enum Level : unsigned char { Debug, Verbose, Info, Error };
   static char indicator(Level L) { return "DVIE"[L]; }
 
   /// Implementations of this method must be thread-safe.
-  virtual void log(Level, const llvm::formatv_object_base &Message) = 0;
+  virtual void log(Level, const char *Fmt,
+   const llvm::formatv_object_base &Message) = 0;
 };
 
 namespace detail {
 const char *debugType(const char *Filename);
-void log(Logger::Level, const llvm::formatv_object_base &);
+void logImpl(Logger::Level, const char *Fmt, const llvm::formatv_object_base &);
 
 // We often want to consume llvm::Errors by value when passing them to log().
 // We automatically wrap them in llvm::fmt_consume() as formatv requires.
@@ -43,7 +46,8 @@
 }
 template 
 void log(Logger::Level L, const char *Fmt, Ts &&... Vals) {
-  detail::log(L, llvm::formatv(Fmt, detail::wrap(std::forward(Vals))...));
+  detail::logImpl(L, Fmt,
+  llvm::formatv(Fmt, detail::wrap(std::forward(Vals))...));
 }
 
 llvm::Error error(std::error_code, std::string &&);
@@ -119,7 +123,8 @@
   : MinLevel(MinLevel), Logs(Logs) {}
 
   /// Write a line to the logging stream.
-  void log(Level, const llvm::formatv_object_base &Message) override;
+  void log(Level, const char *Fmt,
+   const llvm::formatv_object_base &Message) override;
 
 private:
   Logger::Level MinLevel;
Index: clang-tools-extra/clangd/support/Logger.cpp
===
--- clang-tools-extra/clangd/support/Logger.cpp
+++ clang-tools-extra/clangd/support/Logger.cpp
@@ -28,10 +28,10 @@
 
 LoggingSession::~LoggingSession() { L = nullptr; }
 
-void detail::log(Logger::Level Level,
- const llvm::formatv_object_base &Message) {
+void detail::logImpl(Logger::Level Level, const char *Fmt,
+ const llvm::formatv_object_base &Message) {
   if (L)
-L->log(Level, Message);
+L->log(Level, Fmt, Message);
   else {
 static std::mutex Mu;
 std::lock_guard Guard(Mu);
@@ -47,7 +47,7 @@
   return Filename;
 }
 
-void StreamLogger::log(Logger::Level Level,
+void StreamLogger::log(Logger::Level Level, const char *Fmt,
const llvm::formatv_object_base &Message) {
   if (Level < MinLevel)
 return;
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -12,6 +12,7 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "support/Context.h"
 #include "support/Logger.h"
 #include "support/Shutdown.h"
 #include "support/ThreadsafeFS.h"
@@ -23,6 +24,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/VirtualFileSystem.h"
@@ -30,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -58,6 +61,12 @@
 llvm::cl::init(Logger::Info),
 };
 
+llvm::cl::opt LogPublic{
+"log-public",
+llvm::cl::desc("Avoid logging potentially-sensitive request details"),
+llvm::cl::init(false),
+};
+
 llvm::cl::opt TraceFile(
 "trace-file",
 llvm::cl::desc("Path to the fil

[PATCH] D90436: [Bundler] Use argv[0] as the default choice for the Executable name.

[Samuel Antao via Phabricator via cfe-commits]

sfantao added a comment.

@tra, thank you for the patch. This makes sense.

The patch looks good but it has to be someone else to okay it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90436

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

[clang-tools-extra] c29513f - [clangd] Fix check-clangd with no clang built

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-11-02T21:10:43+01:00
New Revision: c29513f7e023f125c6d221db179dc40b79e5c074

URL: 
https://github.com/llvm/llvm-project/commit/c29513f7e023f125c6d221db179dc40b79e5c074
DIFF: 
https://github.com/llvm/llvm-project/commit/c29513f7e023f125c6d221db179dc40b79e5c074.diff

LOG: [clangd] Fix check-clangd with no clang built

- pass required=False to use_clang(), as we don't need it
- fix required=False (which was unused and rotted):
  - make derived substitutions conditional on it
  - add a feature so we can disable tests that need it
- conditionally disable our one test that depends on %resource_dir.
  This doesn't seem right from first principles, but isn't a big deal.

Differential Revision: https://reviews.llvm.org/D90528

Added: 


Modified: 
clang-tools-extra/clangd/test/document-link.test
clang-tools-extra/clangd/test/lit.cfg.py
llvm/utils/lit/lit/llvm/config.py

Removed: 




diff  --git a/clang-tools-extra/clangd/test/document-link.test 
b/clang-tools-extra/clangd/test/document-link.test
index a4c82e5b2fb6..63f8ca099322 100644
--- a/clang-tools-extra/clangd/test/document-link.test
+++ b/clang-tools-extra/clangd/test/document-link.test
@@ -1,3 +1,4 @@
+# for %resource_dir: REQUIRES: clang
 # %resource_dir actually points at builtin_include_dir, go up one directory.
 # RUN: clangd -lit-test -resource-dir=%resource_dir/.. < %s | FileCheck 
-strict-whitespace %s
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

diff  --git a/clang-tools-extra/clangd/test/lit.cfg.py 
b/clang-tools-extra/clangd/test/lit.cfg.py
index a085f6a58dac..a8a38a7fafd3 100644
--- a/clang-tools-extra/clangd/test/lit.cfg.py
+++ b/clang-tools-extra/clangd/test/lit.cfg.py
@@ -1,7 +1,7 @@
 import lit.llvm
 
 lit.llvm.initialize(lit_config, config)
-lit.llvm.llvm_config.use_clang()
+lit.llvm.llvm_config.use_clang([], [], required=False)
 lit.llvm.llvm_config.use_default_substitutions()
 
 config.name = 'Clangd'

diff  --git a/llvm/utils/lit/lit/llvm/config.py 
b/llvm/utils/lit/lit/llvm/config.py
index 8897deb24cde..498e34cae3a5 100644
--- a/llvm/utils/lit/lit/llvm/config.py
+++ b/llvm/utils/lit/lit/llvm/config.py
@@ -396,11 +396,6 @@ def use_clang(self, additional_tool_dirs=[], 
additional_flags=[], required=True)
 
 self.with_environment('LD_LIBRARY_PATH', paths, append_path=True)
 
-# Discover the 'clang' and 'clangcc' to use.
-
-self.config.clang = self.use_llvm_tool(
-'clang', search_env='CLANG', required=required)
-
 shl = getattr(self.config, 'llvm_shlib_dir', None)
 pext = getattr(self.config, 'llvm_plugin_ext', None)
 if shl:
@@ -408,23 +403,28 @@ def use_clang(self, additional_tool_dirs=[], 
additional_flags=[], required=True)
 if pext:
 self.config.substitutions.append(('%pluginext', pext))
 
-builtin_include_dir = 
self.get_clang_builtin_include_dir(self.config.clang)
-tool_substitutions = [
-ToolSubst('%clang', command=self.config.clang, 
extra_args=additional_flags),
-ToolSubst('%clang_analyze_cc1', command='%clang_cc1', 
extra_args=['-analyze', '%analyze', '-setup-static-analyzer']+additional_flags),
-ToolSubst('%clang_cc1', command=self.config.clang, 
extra_args=['-cc1', '-internal-isystem', builtin_include_dir, 
'-nostdsysteminc']+additional_flags),
-ToolSubst('%clang_cpp', command=self.config.clang, 
extra_args=['--driver-mode=cpp']+additional_flags),
-ToolSubst('%clang_cl', command=self.config.clang, 
extra_args=['--driver-mode=cl']+additional_flags),
-ToolSubst('%clangxx', command=self.config.clang, 
extra_args=['--driver-mode=g++']+additional_flags),
-]
-self.add_tool_substitutions(tool_substitutions)
+# Discover the 'clang' and 'clangcc' to use.
+self.config.clang = self.use_llvm_tool(
+'clang', search_env='CLANG', required=required)
+if self.config.clang:
+  self.config.available_features.add('clang')
+  builtin_include_dir = 
self.get_clang_builtin_include_dir(self.config.clang)
+  tool_substitutions = [
+  ToolSubst('%clang', command=self.config.clang, 
extra_args=additional_flags),
+  ToolSubst('%clang_analyze_cc1', command='%clang_cc1', 
extra_args=['-analyze', '%analyze', '-setup-static-analyzer']+additional_flags),
+  ToolSubst('%clang_cc1', command=self.config.clang, 
extra_args=['-cc1', '-internal-isystem', builtin_include_dir, 
'-nostdsysteminc']+additional_flags),
+  ToolSubst('%clang_cpp', command=self.config.clang, 
extra_args=['--driver-mode=cpp']+additional_flags),
+  ToolSubst('%clang_cl', command=self.config.clang, 
extra_args=['--driver-mode=cl']+additional_flags),
+  ToolSubst('%clangx

[PATCH] D90528: [clangd] Fix check-clangd with no clang built

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc29513f7e023: [clangd] Fix check-clangd with no clang built 
(authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D90528?vs=302084&id=302362#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90528

Files:
  clang-tools-extra/clangd/test/document-link.test
  clang-tools-extra/clangd/test/lit.cfg.py
  llvm/utils/lit/lit/llvm/config.py


Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -396,11 +396,6 @@
 
 self.with_environment('LD_LIBRARY_PATH', paths, append_path=True)
 
-# Discover the 'clang' and 'clangcc' to use.
-
-self.config.clang = self.use_llvm_tool(
-'clang', search_env='CLANG', required=required)
-
 shl = getattr(self.config, 'llvm_shlib_dir', None)
 pext = getattr(self.config, 'llvm_plugin_ext', None)
 if shl:
@@ -408,23 +403,28 @@
 if pext:
 self.config.substitutions.append(('%pluginext', pext))
 
-builtin_include_dir = 
self.get_clang_builtin_include_dir(self.config.clang)
-tool_substitutions = [
-ToolSubst('%clang', command=self.config.clang, 
extra_args=additional_flags),
-ToolSubst('%clang_analyze_cc1', command='%clang_cc1', 
extra_args=['-analyze', '%analyze', '-setup-static-analyzer']+additional_flags),
-ToolSubst('%clang_cc1', command=self.config.clang, 
extra_args=['-cc1', '-internal-isystem', builtin_include_dir, 
'-nostdsysteminc']+additional_flags),
-ToolSubst('%clang_cpp', command=self.config.clang, 
extra_args=['--driver-mode=cpp']+additional_flags),
-ToolSubst('%clang_cl', command=self.config.clang, 
extra_args=['--driver-mode=cl']+additional_flags),
-ToolSubst('%clangxx', command=self.config.clang, 
extra_args=['--driver-mode=g++']+additional_flags),
-]
-self.add_tool_substitutions(tool_substitutions)
+# Discover the 'clang' and 'clangcc' to use.
+self.config.clang = self.use_llvm_tool(
+'clang', search_env='CLANG', required=required)
+if self.config.clang:
+  self.config.available_features.add('clang')
+  builtin_include_dir = 
self.get_clang_builtin_include_dir(self.config.clang)
+  tool_substitutions = [
+  ToolSubst('%clang', command=self.config.clang, 
extra_args=additional_flags),
+  ToolSubst('%clang_analyze_cc1', command='%clang_cc1', 
extra_args=['-analyze', '%analyze', '-setup-static-analyzer']+additional_flags),
+  ToolSubst('%clang_cc1', command=self.config.clang, 
extra_args=['-cc1', '-internal-isystem', builtin_include_dir, 
'-nostdsysteminc']+additional_flags),
+  ToolSubst('%clang_cpp', command=self.config.clang, 
extra_args=['--driver-mode=cpp']+additional_flags),
+  ToolSubst('%clang_cl', command=self.config.clang, 
extra_args=['--driver-mode=cl']+additional_flags),
+  ToolSubst('%clangxx', command=self.config.clang, 
extra_args=['--driver-mode=g++']+additional_flags),
+  ]
+  self.add_tool_substitutions(tool_substitutions)
+  self.config.substitutions.append(
+  ('%resource_dir', builtin_include_dir))
 
 self.config.substitutions.append(('%itanium_abi_triple',
   
self.make_itanium_abi_triple(self.config.target_triple)))
 self.config.substitutions.append(('%ms_abi_triple',
   
self.make_msabi_triple(self.config.target_triple)))
-self.config.substitutions.append(
-('%resource_dir', builtin_include_dir))
 
 # The host triple might not be set, at least if we're compiling clang 
from
 # an already installed llvm.
Index: clang-tools-extra/clangd/test/lit.cfg.py
===
--- clang-tools-extra/clangd/test/lit.cfg.py
+++ clang-tools-extra/clangd/test/lit.cfg.py
@@ -1,7 +1,7 @@
 import lit.llvm
 
 lit.llvm.initialize(lit_config, config)
-lit.llvm.llvm_config.use_clang()
+lit.llvm.llvm_config.use_clang([], [], required=False)
 lit.llvm.llvm_config.use_default_substitutions()
 
 config.name = 'Clangd'
Index: clang-tools-extra/clangd/test/document-link.test
===
--- clang-tools-extra/clangd/test/document-link.test
+++ clang-tools-extra/clangd/test/document-link.test
@@ -1,3 +1,4 @@
+# for %resource_dir: REQUIRES: clang
 # %resource_dir actually points at builtin_include_dir, go up one directory.
 # RUN: clangd -lit-test -resource-dir=%resource_dir/.. < %s | FileCheck 
-strict-whit

[clang] 9f151df - Change Module::ASTFile and ModuleFile::File => Optional, NFC

[Duncan P . N . Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-11-02T15:11:51-05:00
New Revision: 9f151df17800e1668c32e5314a290ae94c8f2dd3

URL: 
https://github.com/llvm/llvm-project/commit/9f151df17800e1668c32e5314a290ae94c8f2dd3
DIFF: 
https://github.com/llvm/llvm-project/commit/9f151df17800e1668c32e5314a290ae94c8f2dd3.diff

LOG: Change Module::ASTFile and ModuleFile::File => Optional, NFC

Change `Module::ASTFile` and `ModuleFile::File` to use
`Optional` instead of `const FileEntry *`. One of many
steps toward removing `FileEntry::getName`.

Differential Revision: https://reviews.llvm.org/D89836

Added: 


Modified: 
clang/include/clang/Basic/FileEntry.h
clang/include/clang/Basic/Module.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/Basic/Module.cpp
clang/lib/Sema/SemaModule.cpp
clang/lib/Serialization/ModuleManager.cpp
clang/tools/libclang/CIndex.cpp
clang/tools/libclang/CXIndexDataConsumer.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/FileEntry.h 
b/clang/include/clang/Basic/FileEntry.h
index 7f5efb1c48f3..e4c2e7e34db9 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -64,6 +64,7 @@ class FileEntryRef {
   inline unsigned getUID() const;
   inline const llvm::sys::fs::UniqueID &getUniqueID() const;
   inline time_t getModificationTime() const;
+  inline void closeFile() const;
 
   /// Check if the underlying FileEntry is the same, intentially ignoring
   /// whether the file was referenced with the same spelling of the filename.
@@ -360,6 +361,8 @@ time_t FileEntryRef::getModificationTime() const {
   return getFileEntry().getModificationTime();
 }
 
+void FileEntryRef::closeFile() const { getFileEntry().closeFile(); }
+
 } // end namespace clang
 
 #endif // LLVM_CLANG_BASIC_FILEENTRY_H

diff  --git a/clang/include/clang/Basic/Module.h 
b/clang/include/clang/Basic/Module.h
index ac33c7573f35..5a4975f25b72 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -15,6 +15,7 @@
 #ifndef LLVM_CLANG_BASIC_MODULE_H
 #define LLVM_CLANG_BASIC_MODULE_H
 
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
@@ -160,7 +161,7 @@ class Module {
 
   /// The AST file if this is a top-level module which has a
   /// corresponding serialized AST file, or null otherwise.
-  const FileEntry *ASTFile = nullptr;
+  Optional ASTFile;
 
   /// The top-level headers associated with this module.
   llvm::SmallSetVector TopHeaders;
@@ -529,14 +530,14 @@ class Module {
   }
 
   /// The serialized AST file for this module, if one was created.
-  const FileEntry *getASTFile() const {
+  OptionalFileEntryRefDegradesToFileEntryPtr getASTFile() const {
 return getTopLevelModule()->ASTFile;
   }
 
   /// Set the serialized AST file for the top-level module of this module.
-  void setASTFile(const FileEntry *File) {
-assert((File == nullptr || getASTFile() == nullptr ||
-getASTFile() == File) && "file path changed");
+  void setASTFile(Optional File) {
+assert((!File || !getASTFile() || getASTFile() == File) &&
+   "file path changed");
 getTopLevelModule()->ASTFile = File;
   }
 

diff  --git a/clang/include/clang/Serialization/ModuleFile.h 
b/clang/include/clang/Serialization/ModuleFile.h
index 598e61210702..a309c1143350 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -159,7 +159,7 @@ class ModuleFile {
   bool DidReadTopLevelSubmodule = false;
 
   /// The file entry for the module file.
-  const FileEntry *File = nullptr;
+  OptionalFileEntryRefDegradesToFileEntryPtr File;
 
   /// The signature of the module file, which may be used instead of the size
   /// and modification time to identify this particular file.

diff  --git a/clang/include/clang/Serialization/ModuleManager.h 
b/clang/include/clang/Serialization/ModuleManager.h
index 15ddb9875ff1..7081eedad4b4 100644
--- a/clang/include/clang/Serialization/ModuleManager.h
+++ b/clang/include/clang/Serialization/ModuleManager.h
@@ -307,10 +307,8 @@ class ModuleManager {
   /// \returns True if a file exists but does not meet the size/
   /// modification time criteria, false if the file is either available and
   /// suitable, or is missing.
-  bool lookupModuleFile(StringRef FileName,
-off_t ExpectedSize,
-time_t ExpectedModTime,
-const FileEntry *&File);
+  bool lookupModuleFile(StringRef FileName, off_t ExpectedSize,
+time_t ExpectedModTime, Optional &File);
 
   /// View the graphviz representation of the module graph.
   void viewGraph();

diff  --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index b25248de6832..6