[PATCH] D61729: [docs] Fix example for Allman brace breaking style

[Jan Korous via Phabricator via cfe-commits]

jkorous edited reviewers, added: owenpan; removed: llvm-commits.
jkorous added a subscriber: owenpan.
jkorous added a comment.

You're right, thanks for letting me know.

It was changed in 806d5741aa7f 
.

@owenpan was that intended?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61729



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

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D62761#1525917 , @NoQ wrote:

> Remove "-" from program point dumps because it resembles a removal marker in 
> diffs.


Could you add an image? I have not seen any problematic stuff, just that.


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

https://reviews.llvm.org/D62761



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

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Csaba Dabis via Phabricator via cfe-commits]

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

Let us see those tests!


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

https://reviews.llvm.org/D62638



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

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 202514.
NoQ added a comment.

Remove "-" from program point dumps because it resembles a removal marker in 
diffs.


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

https://reviews.llvm.org/D62761

Files:
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -9,6 +9,13 @@
 import re
 
 
+# A helper function for finding the difference between two dictionaries.
+def diff_dicts(curr, prev):
+removed = [k for k in prev if k not in curr or curr[k] != prev[k]]
+added = [k for k in curr if k not in prev or curr[k] != prev[k]]
+return (removed, added)
+
+
 # A deserialized source location.
 class SourceLocation(object):
 def __init__(self, json_loc):
@@ -38,13 +45,21 @@
 self.block_id = json_pp['block_id']
 
 
-# A value of a single expression in a deserialized Environment.
-class EnvironmentBinding(object):
-def __init__(self, json_eb):
-super(EnvironmentBinding, self).__init__()
-self.stmt_id = json_eb['stmt_id']
-self.pretty = json_eb['pretty']
-self.value = json_eb['value']
+# A single expression acting as a key in a deserialized Environment.
+class EnvironmentBindingKey(object):
+def __init__(self, json_ek):
+super(EnvironmentBindingKey, self).__init__()
+self.stmt_id = json_ek['stmt_id']
+self.pretty = json_ek['pretty']
+
+def _key(self):
+return self.stmt_id
+
+def __eq__(self, other):
+return self._key() == other._key()
+
+def __hash__(self):
+return hash(self._key())
 
 
 # Deserialized description of a location context.
@@ -56,6 +71,15 @@
 self.decl = json_frame['calling']
 self.line = json_frame['call_line']
 
+def _key(self):
+return self.lctx_id
+
+def __eq__(self, other):
+return self._key() == other._key()
+
+def __hash__(self):
+return hash(self._key())
+
 
 # A group of deserialized Environment bindings that correspond to a specific
 # location context.
@@ -63,8 +87,17 @@
 def __init__(self, json_frame):
 super(EnvironmentFrame, self).__init__()
 self.location_context = LocationContext(json_frame)
-self.bindings = [EnvironmentBinding(b) for b in json_frame['items']] \
-if json_frame['items'] is not None else []
+self.bindings = collections.OrderedDict(
+[(EnvironmentBindingKey(b),
+  b['value']) for b in json_frame['items']]
+if json_frame['items'] is not None else [])
+
+def diff_bindings(self, prev):
+return diff_dicts(self.bindings, prev.bindings)
+
+def is_different(self, prev):
+removed, added = self.diff_bindings(prev)
+return len(removed) != 0 or len(added) != 0
 
 
 # A deserialized Environment.
@@ -73,29 +106,80 @@
 super(Environment, self).__init__()
 self.frames = [EnvironmentFrame(f) for f in json_e]
 
+def diff_frames(self, prev):
+# TODO: It's difficult to display a good diff when frame numbers shift.
+if len(self.frames) != len(prev.frames):
+return None
+
+updated = []
+for i in range(len(self.frames)):
+f = self.frames[i]
+prev_f = prev.frames[i]
+if f.location_context == prev_f.location_context:
+if f.is_different(prev_f):
+updated.append(i)
+else:
+# We have the whole frame replaced with another frame.
+# TODO: Produce a nice diff.
+return None
+
+# TODO: Add support for added/removed.
+return updated
+
+def is_different(self, prev):
+updated = self.diff_frames(prev)
+return updated is None or len(updated) > 0
+
+
+# A single binding key in a deserialized RegionStore cluster.
+class StoreBindingKey(object):
+def __init__(self, json_sk):
+super(StoreBindingKey, self).__init__()
+self.kind = json_sk['kind']
+self.offset = json_sk['offset']
 
-# A single binding in a deserialized RegionStore cluster.
-class StoreBinding(object):
-def __init__(self, json_sb):
-super(StoreBinding, self).__init__()
-self.kind = json_sb['kind']
-self.offset = json_sb['offset']
-self.value = json_sb['value']
+def _key(self):
+return (self.kind, self.offset)
+
+def __eq__(self, other):
+return self._key() == other._key()
+
+def __hash__(self):
+return hash(self._key())
 
 
 # A single cluster of the deserialized RegionStore.
 class StoreCluster(object):
 def __init__(self, json_sc):
 super(StoreCluster, self).__init__()
-self.base_region = json_sc['cluster']
-self.bindings = 

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I'll add some tests as soon as i'm sure tests for this thing actually work (by 
landing D62638 ).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62761



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

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.
NoQ added a comment.
NoQ added a parent revision: D62638: [analyzer] A Python script to prettify the 
ExplodedGraph dumps..

I'll add some tests as soon as i'm sure tests for this thing actually work (by 
landing D62638 ).


Implement a mode in which the rewriter prints differences between adjacent 
program states rather than the whole states. The whole state is still printed 
for nodes with multiple predecessors (because merge diffs were annoying to 
implement and it's still nice to have the full state occasionally) and for leaf 
nodes (because they're the important ones).

F8999355: Screen Shot 2019-05-31 at 4.57.55 PM.png 


The diffs are computed "semantically" as opposed to plain text diffs. I.e., the 
diff algorithm is hand-crafted separately for every state trait, taking the 
underlying data structures into account. This is especially nice for 
`Environment` because textual diffs would have been terrible. This, of course, 
produces quite some boilerplate, but i think it's isolated enough to not bother 
me that much.


Repository:
  rC Clang

https://reviews.llvm.org/D62761

Files:
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -9,6 +9,13 @@
 import re
 
 
+# A helper function for finding the difference between two dictionaries.
+def diff_dicts(curr, prev):
+removed = [k for k in prev if k not in curr or curr[k] != prev[k]]
+added = [k for k in curr if k not in prev or curr[k] != prev[k]]
+return (removed, added)
+
+
 # A deserialized source location.
 class SourceLocation(object):
 def __init__(self, json_loc):
@@ -38,13 +45,21 @@
 self.block_id = json_pp['block_id']
 
 
-# A value of a single expression in a deserialized Environment.
-class EnvironmentBinding(object):
-def __init__(self, json_eb):
-super(EnvironmentBinding, self).__init__()
-self.stmt_id = json_eb['stmt_id']
-self.pretty = json_eb['pretty']
-self.value = json_eb['value']
+# A single expression acting as a key in a deserialized Environment.
+class EnvironmentBindingKey(object):
+def __init__(self, json_ek):
+super(EnvironmentBindingKey, self).__init__()
+self.stmt_id = json_ek['stmt_id']
+self.pretty = json_ek['pretty']
+
+def _key(self):
+return self.stmt_id
+
+def __eq__(self, other):
+return self._key() == other._key()
+
+def __hash__(self):
+return hash(self._key())
 
 
 # Deserialized description of a location context.
@@ -56,6 +71,15 @@
 self.decl = json_frame['calling']
 self.line = json_frame['call_line']
 
+def _key(self):
+return self.lctx_id
+
+def __eq__(self, other):
+return self._key() == other._key()
+
+def __hash__(self):
+return hash(self._key())
+
 
 # A group of deserialized Environment bindings that correspond to a specific
 # location context.
@@ -63,8 +87,17 @@
 def __init__(self, json_frame):
 super(EnvironmentFrame, self).__init__()
 self.location_context = LocationContext(json_frame)
-self.bindings = [EnvironmentBinding(b) for b in json_frame['items']] \
-if json_frame['items'] is not None else []
+self.bindings = collections.OrderedDict(
+[(EnvironmentBindingKey(b),
+  b['value']) for b in json_frame['items']]
+if json_frame['items'] is not None else [])
+
+def diff_bindings(self, prev):
+return diff_dicts(self.bindings, prev.bindings)
+
+def is_different(self, prev):
+removed, added = self.diff_bindings(prev)
+return len(removed) != 0 or len(added) != 0
 
 
 # A deserialized Environment.
@@ -73,29 +106,80 @@
 super(Environment, self).__init__()
 self.frames = [EnvironmentFrame(f) for f in json_e]
 
+def diff_frames(self, prev):
+# TODO: It's difficult to display a good diff when frame numbers shift.
+if len(self.frames) != len(prev.frames):
+return None
+
+updated = []
+for i in range(len(self.frames)):
+f = self.frames[i]
+prev_f = prev.frames[i]
+if f.location_context == prev_f.location_context:
+if f.is_different(prev_f):
+updated.append(i)
+else:
+# We have the whole frame replaced with another frame.
+# TODO: Produce a nice diff.
+return None
+
+# TODO: Add 

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 202511.
NoQ added a comment.

Rebase.


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

https://reviews.llvm.org/D62638

Files:
  clang/test/Analysis/exploded-graph-rewriter/edge.dot
  clang/test/Analysis/exploded-graph-rewriter/empty.dot
  clang/test/Analysis/exploded-graph-rewriter/environment.dot
  clang/test/Analysis/exploded-graph-rewriter/lit.local.cfg
  clang/test/Analysis/exploded-graph-rewriter/program_points.dot
  clang/test/Analysis/exploded-graph-rewriter/store.dot
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- /dev/null
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -0,0 +1,398 @@
+#!/usr/bin/env python
+
+from __future__ import print_function
+
+import argparse
+import collections
+import json
+import logging
+import re
+
+
+# A deserialized source location.
+class SourceLocation(object):
+def __init__(self, json_loc):
+super(SourceLocation, self).__init__()
+self.line = json_loc['line']
+self.col = json_loc['column']
+self.filename = json_loc['filename'] \
+if 'filename' in json_loc else '(main file)'
+
+
+# A deserialized program point.
+class ProgramPoint(object):
+def __init__(self, json_pp):
+super(ProgramPoint, self).__init__()
+self.kind = json_pp['kind']
+self.tag = json_pp['tag']
+if self.kind == 'Edge':
+self.src_id = json_pp['src_id']
+self.dst_id = json_pp['dst_id']
+elif self.kind == 'Statement':
+self.stmt_kind = json_pp['stmt_kind']
+self.pointer = json_pp['pointer']
+self.pretty = json_pp['pretty']
+self.loc = SourceLocation(json_pp['location']) \
+if json_pp['location'] is not None else None
+elif self.kind == 'BlockEntrance':
+self.block_id = json_pp['block_id']
+
+
+# A value of a single expression in a deserialized Environment.
+class EnvironmentBinding(object):
+def __init__(self, json_eb):
+super(EnvironmentBinding, self).__init__()
+self.stmt_id = json_eb['stmt_id']
+self.pretty = json_eb['pretty']
+self.value = json_eb['value']
+
+
+# Deserialized description of a location context.
+class LocationContext(object):
+def __init__(self, json_frame):
+super(LocationContext, self).__init__()
+self.lctx_id = json_frame['lctx_id']
+self.caption = json_frame['location_context']
+self.decl = json_frame['calling']
+self.line = json_frame['call_line']
+
+
+# A group of deserialized Environment bindings that correspond to a specific
+# location context.
+class EnvironmentFrame(object):
+def __init__(self, json_frame):
+super(EnvironmentFrame, self).__init__()
+self.location_context = LocationContext(json_frame)
+self.bindings = [EnvironmentBinding(b) for b in json_frame['items']] \
+if json_frame['items'] is not None else []
+
+
+# A deserialized Environment.
+class Environment(object):
+def __init__(self, json_e):
+super(Environment, self).__init__()
+self.frames = [EnvironmentFrame(f) for f in json_e]
+
+
+# A single binding in a deserialized RegionStore cluster.
+class StoreBinding(object):
+def __init__(self, json_sb):
+super(StoreBinding, self).__init__()
+self.kind = json_sb['kind']
+self.offset = json_sb['offset']
+self.value = json_sb['value']
+
+
+# A single cluster of the deserialized RegionStore.
+class StoreCluster(object):
+def __init__(self, json_sc):
+super(StoreCluster, self).__init__()
+self.base_region = json_sc['cluster']
+self.bindings = [StoreBinding(b) for b in json_sc['items']]
+
+
+# A deserialized RegionStore.
+class Store(object):
+def __init__(self, json_s):
+super(Store, self).__init__()
+self.clusters = [StoreCluster(c) for c in json_s]
+
+
+# A deserialized program state.
+class ProgramState(object):
+def __init__(self, state_id, json_ps):
+super(ProgramState, self).__init__()
+logging.debug('Adding ProgramState ' + str(state_id))
+
+self.state_id = state_id
+self.store = Store(json_ps['store']) \
+if json_ps['store'] is not None else None
+self.environment = Environment(json_ps['environment']) \
+if json_ps['environment'] is not None else None
+# TODO: Objects under construction.
+# TODO: Constraint ranges.
+# TODO: Dynamic types of objects.
+# TODO: Checker messages.
+
+
+# A deserialized exploded graph node. Has a default constructor because it
+# may be referenced as part of an edge before its contents are deserialized,
+# and in this moment we already need a room for predecessors and successors.
+class ExplodedNode(object):
+def 

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:191
+.replace('\\>', '>') \
+.rstrip(',')
+logging.debug(raw_json)

Charusso wrote:
> NoQ wrote:
> > Charusso wrote:
> > > NoQ wrote:
> > > > Charusso wrote:
> > > > > I have removed the trailing `,` from the end yesterday so 
> > > > > `rstrip(',')` is not needed.
> > > > > 
> > > > > It would be cool to name the `result[]`, like: `pred_id = result[1]` 
> > > > > and `succ_id = result[2]` or current/previous ID, or something like 
> > > > > that.
> > > > > 
> > > > > I also would put the regexes into a function and you could write:
> > > > > `pred_id, succ_id = parse_edge()` or something more simpler.
> > > > > 
> > > > > What is `result[0]` btw? That confused me a little-bit.
> > > > > `pred_id, succ_id = parse_edge()`
> > > > 
> > > > Not really, i'd also have to return whether the match was successful, 
> > > > which kinda makes it as bad as before.
> > > > 
> > > > > What is `result[0]` btw? That confused me a little-bit.
> > > > 
> > > > That's the entire match. 
> > > > https://docs.python.org/3/library/re.html#match-objects
> > > I see, just I am that newbie. Thanks!
> > > I have removed the trailing `,` from the end yesterday so `rstrip(',')` 
> > > is not needed.
> > 
> > It still seems to be there for all nodes except the last node.
> > 
> > 
> Hm, that is why your testing idea is so cool, I am nearly sure I have removed 
> it.
//*politely refuses to take credit for inventing test-driven development*//


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

https://reviews.llvm.org/D62638



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

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:191
+.replace('\\>', '>') \
+.rstrip(',')
+logging.debug(raw_json)

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Charusso wrote:
> > > > I have removed the trailing `,` from the end yesterday so `rstrip(',')` 
> > > > is not needed.
> > > > 
> > > > It would be cool to name the `result[]`, like: `pred_id = result[1]` 
> > > > and `succ_id = result[2]` or current/previous ID, or something like 
> > > > that.
> > > > 
> > > > I also would put the regexes into a function and you could write:
> > > > `pred_id, succ_id = parse_edge()` or something more simpler.
> > > > 
> > > > What is `result[0]` btw? That confused me a little-bit.
> > > > `pred_id, succ_id = parse_edge()`
> > > 
> > > Not really, i'd also have to return whether the match was successful, 
> > > which kinda makes it as bad as before.
> > > 
> > > > What is `result[0]` btw? That confused me a little-bit.
> > > 
> > > That's the entire match. 
> > > https://docs.python.org/3/library/re.html#match-objects
> > I see, just I am that newbie. Thanks!
> > I have removed the trailing `,` from the end yesterday so `rstrip(',')` is 
> > not needed.
> 
> It still seems to be there for all nodes except the last node.
> 
> 
Hm, that is why your testing idea is so cool, I am nearly sure I have removed 
it.


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

https://reviews.llvm.org/D62638



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

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:191
+.replace('\\>', '>') \
+.rstrip(',')
+logging.debug(raw_json)

Charusso wrote:
> NoQ wrote:
> > Charusso wrote:
> > > I have removed the trailing `,` from the end yesterday so `rstrip(',')` 
> > > is not needed.
> > > 
> > > It would be cool to name the `result[]`, like: `pred_id = result[1]` and 
> > > `succ_id = result[2]` or current/previous ID, or something like that.
> > > 
> > > I also would put the regexes into a function and you could write:
> > > `pred_id, succ_id = parse_edge()` or something more simpler.
> > > 
> > > What is `result[0]` btw? That confused me a little-bit.
> > > `pred_id, succ_id = parse_edge()`
> > 
> > Not really, i'd also have to return whether the match was successful, which 
> > kinda makes it as bad as before.
> > 
> > > What is `result[0]` btw? That confused me a little-bit.
> > 
> > That's the entire match. 
> > https://docs.python.org/3/library/re.html#match-objects
> I see, just I am that newbie. Thanks!
> I have removed the trailing `,` from the end yesterday so `rstrip(',')` is 
> not needed.

It still seems to be there for all nodes except the last node.




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

https://reviews.llvm.org/D62638



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

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D62638#1525843 , @Charusso wrote:

> In D62638#1525823 , @NoQ wrote:
>
> > Also switched to python2 because it seems to be the default. The 
> > differences are minimal.
>
>
> What about the state of the patch itself? Planned stuff WIP, could I take it 
> to SVGify, or?


The format would look roughly the same from now on, but i'll add more prints 
for other state traits and may slightly rearrange fields around, also diffs 
which would be simply extra +/- fields in the tables that otherwise look the 
same. I guess you should wait until i commit this revision and we at least 
figure out what's the situation with buildbots and tests.


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

https://reviews.llvm.org/D62638



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

[PATCH] D62045: Revise the google-objc-global-variable-declaration check to match the style guide.

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362279: Revise the google-objc-global-variable-declaration 
check to match the style… (authored by stephanemoore, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62045?vs=201973=202508#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62045

Files:
  clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  
clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m

Index: clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -23,29 +23,35 @@
 
 namespace {
 
-AST_MATCHER(VarDecl, isLocalVariable) {
-  return Node.isLocalVarDecl();
-}
+AST_MATCHER(VarDecl, isLocalVariable) { return Node.isLocalVarDecl(); }
 
 FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) {
+  if (IsConst && (Decl->getStorageClass() != SC_Static)) {
+// No fix available if it is not a static constant, since it is difficult
+// to determine the proper fix in this case.
+return FixItHint();
+  }
+
   char FC = Decl->getName()[0];
   if (!llvm::isAlpha(FC) || Decl->getName().size() == 1) {
 // No fix available if first character is not alphabetical character, or it
-// is a single-character variable, since it is difficult to determine the 
+// is a single-character variable, since it is difficult to determine the
 // proper fix in this case. Users should create a proper variable name by
 // their own.
 return FixItHint();
   }
   char SC = Decl->getName()[1];
   if ((FC == 'k' || FC == 'g') && !llvm::isAlpha(SC)) {
-// No fix available if the prefix is correct but the second character is not
-// alphabetical, since it is difficult to determine the proper fix in this
-// case.
+// No fix available if the prefix is correct but the second character is
+// not alphabetical, since it is difficult to determine the proper fix in
+// this case.
 return FixItHint();
   }
+
   auto NewName = (IsConst ? "k" : "g") +
  llvm::StringRef(std::string(1, FC)).upper() +
  Decl->getName().substr(1).str();
+
   return FixItHint::CreateReplacement(
   CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
   llvm::StringRef(NewName));
@@ -71,7 +77,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
+ unless(matchesName("::(k[A-Z])|([A-Z][A-Z0-9])")))
  .bind("global_const"),
  this);
 }
Index: clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
===
--- clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
+++ clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
@@ -1,10 +1,14 @@
 // RUN: %check_clang_tidy %s google-objc-global-variable-declaration %t
 
 @class NSString;
+
 static NSString* const myConstString = @"hello";
 // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'myConstString' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const kMyConstString = @"hello";
 
+extern NSString* const GlobalConstant = @"hey";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'GlobalConstant' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
+
 static NSString* MyString = @"hi";
 // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: non-const global variable 'MyString' must have a name which starts with 'g[A-Z]' [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* gMyString = @"hi";
@@ -25,12 +29,25 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const _notAlpha = @"NotBeginWithAlpha";
 
+static NSString* const notCap = @"NotBeginWithCap";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'notCap' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
+// CHECK-FIXES: static NSString* const kNotCap = @"NotBeginWithCap";
+
 static NSString* const k_Alpha = @"SecondNotAlpha";
 // CHECK-MESSAGES: 

[clang-tools-extra] r362279 - Revise the google-objc-global-variable-declaration check to match the style guide.

[Stephane Moore via cfe-commits]

Author: stephanemoore
Date: Fri May 31 16:41:15 2019
New Revision: 362279

URL: http://llvm.org/viewvc/llvm-project?rev=362279=rev
Log:
Revise the google-objc-global-variable-declaration check to match the style 
guide.

Summary:
Revise the google-objc-global-variable-declaration check to match the style 
guide.

This commit updates the check as follows:
(1) Do not emit fixes for extern global constants.
(2) Allow the second character of prefixes for constants to be numeric (the new 
guideline is that global constants should generally be named with a prefix that 
begins with a capital letter followed by one or more capital letters or 
numbers).

https://google.github.io/styleguide/objcguide.html#prefixes

This is an amended re-submission of 
https://reviews.llvm.org/rG12e3726fadb0b2a4d8aeed0a2817b5159f9d029d.

Contributed By: yaqiji

Reviewers: Wizard, benhamilton, stephanemoore

Reviewed By: benhamilton, stephanemoore

Subscribers: mgorny, cfe-commits, yaqiji

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m

Modified: 
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp?rev=362279=362278=362279=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/google/GlobalVariableDeclarationCheck.cpp 
Fri May 31 16:41:15 2019
@@ -23,29 +23,35 @@ namespace objc {
 
 namespace {
 
-AST_MATCHER(VarDecl, isLocalVariable) {
-  return Node.isLocalVarDecl();
-}
+AST_MATCHER(VarDecl, isLocalVariable) { return Node.isLocalVarDecl(); }
 
 FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) {
+  if (IsConst && (Decl->getStorageClass() != SC_Static)) {
+// No fix available if it is not a static constant, since it is difficult
+// to determine the proper fix in this case.
+return FixItHint();
+  }
+
   char FC = Decl->getName()[0];
   if (!llvm::isAlpha(FC) || Decl->getName().size() == 1) {
 // No fix available if first character is not alphabetical character, or it
-// is a single-character variable, since it is difficult to determine the 
+// is a single-character variable, since it is difficult to determine the
 // proper fix in this case. Users should create a proper variable name by
 // their own.
 return FixItHint();
   }
   char SC = Decl->getName()[1];
   if ((FC == 'k' || FC == 'g') && !llvm::isAlpha(SC)) {
-// No fix available if the prefix is correct but the second character is 
not
-// alphabetical, since it is difficult to determine the proper fix in this
-// case.
+// No fix available if the prefix is correct but the second character is
+// not alphabetical, since it is difficult to determine the proper fix in
+// this case.
 return FixItHint();
   }
+
   auto NewName = (IsConst ? "k" : "g") +
  llvm::StringRef(std::string(1, FC)).upper() +
  Decl->getName().substr(1).str();
+
   return FixItHint::CreateReplacement(
   CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
   llvm::StringRef(NewName));
@@ -71,7 +77,7 @@ void GlobalVariableDeclarationCheck::reg
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
+ unless(matchesName("::(k[A-Z])|([A-Z][A-Z0-9])")))
  .bind("global_const"),
  this);
 }

Modified: 
clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m?rev=362279=362278=362279=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/google-objc-global-variable-declaration.m
 Fri May 31 16:41:15 2019
@@ -1,10 +1,14 @@
 // RUN: %check_clang_tidy %s google-objc-global-variable-declaration %t
 
 @class NSString;
+
 static NSString* const myConstString = @"hello";
 // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 
'myConstString' must have a name which starts with an appropriate prefix 
[google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const kMyConstString = @"hello";
 
+extern NSString* const GlobalConstant = @"hey";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

Eugene.Zelenko wrote:
> gribozavr wrote:
> > Eugene.Zelenko wrote:
> > > gribozavr wrote:
> > > > Eugene.Zelenko wrote:
> > > > > gribozavr wrote:
> > > > > > Eugene.Zelenko wrote:
> > > > > > > Return type is not mentioned explicitly, so auto should not be 
> > > > > > > used.
> > > > > > An explicit type is not needed for readability here.  The rule is 
> > > > > > to use auto when it improves readability, not when the type is not 
> > > > > > spelled in immediate vicinity.
> > > > > I think it's reasonable to follow modernize-use-auto.
> > > > modernize-use-auto is only a heuristic.
> > > But set of processed situations are very reasonable.
> > In abstract it might sound reasonable.  In practice it is still a heuristic 
> > and not a law.
> I think it's reasonable to keep in memory that not everybody keeps 
> functions/methods' return types in memory.
'auto' and the rules about 'auto' serve readability purposes. They are not for 
ensuring that types are always visible in the source code. Knowing the type is 
not the goal in itself. The types in the source code need to serve a purpose, 
like everything else we write. If you think that 'auto' is not reasonable here, 
I would ask you to explain why the code becomes less readable for an average 
developer familiar with Clang -- who knows what constructors are, and what 
constructor arguments are.

As an illustration, consider this code:

```
f(g());
```

We write code like this all the time and we are OK with not seeing the return 
type of `g` in the source code.

If we extract a variable to give it a descriptive name, we should be OK with 
'auto' in:

```
auto descriptiveName = g();
f(x);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

Re: r356569 - [NFC][clang][astdump] Some baseline tests for OpenMP

[Roman Lebedev via cfe-commits]

On Sat, Jun 1, 2019 at 2:00 AM Richard Smith  wrote:
>
> These are change detector tests 
> (https://testing.googleblog.com/2015/01/testing-on-toilet-change-detector-tests.html)
>  and create a significant maintenance burden that exceeds their value.
>
> Please either reduce these to tests that actually test specific salient 
> properties (rather than just being assertions that the AST does not change) 
> or remove them.
Yep, lack of tooling indeed always makes things rather frustrating.
I have recently highlighted that in yet another AST thread:
https://lists.llvm.org/pipermail/cfe-dev/2019-April/061997.html

I personally added those tests to highlight the openmp_structured_block
changes, but there is also unit-test coverage for that.

If maintaining test coverage turns out to be too problematic,
feel free to degrade the test coverage as you see fit,
but i stand by my opinion that it is a preexisting tooling issue.
(same with clang codegen tests, diags, etc etc)

Roman.

> On Wed, 20 Mar 2019 at 11:16, Roman Lebedev via cfe-commits 
>  wrote:
>>
>> Author: lebedevri
>> Date: Wed Mar 20 09:31:47 2019
>> New Revision: 356569
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=356569=rev
>> Log:
>> [NFC][clang][astdump] Some baseline tests for OpenMP
>>
>> Summary:
>> Split off from D59214.
>> Not a fully exhaustive test coverage, but better than what there currently 
>> is.
>>
>> Differential Revision: https://reviews.llvm.org/D59306
>>
>> Added:
>> cfe/trunk/test/AST/ast-dump-openmp-atomic.c
>> cfe/trunk/test/AST/ast-dump-openmp-barrier.c
>> cfe/trunk/test/AST/ast-dump-openmp-cancel.c
>> cfe/trunk/test/AST/ast-dump-openmp-cancellation-point.c
>> cfe/trunk/test/AST/ast-dump-openmp-critical.c
>> cfe/trunk/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-distribute-parallel-for.c
>> cfe/trunk/test/AST/ast-dump-openmp-distribute-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-distribute.c
>> cfe/trunk/test/AST/ast-dump-openmp-flush.c
>> cfe/trunk/test/AST/ast-dump-openmp-for-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-for.c
>> cfe/trunk/test/AST/ast-dump-openmp-master.c
>> cfe/trunk/test/AST/ast-dump-openmp-ordered.c
>> cfe/trunk/test/AST/ast-dump-openmp-parallel-for-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-parallel-for.c
>> cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c
>> cfe/trunk/test/AST/ast-dump-openmp-parallel-sections.c
>> cfe/trunk/test/AST/ast-dump-openmp-parallel.c
>> cfe/trunk/test/AST/ast-dump-openmp-section.c
>> cfe/trunk/test/AST/ast-dump-openmp-sections.c
>> cfe/trunk/test/AST/ast-dump-openmp-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-single.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-data.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-enter-data.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-exit-data.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-parallel-for-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-parallel-for.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-parallel.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-simd.c
>> 
>> cfe/trunk/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-teams-distribute.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-teams.c
>> cfe/trunk/test/AST/ast-dump-openmp-target-update.c
>> cfe/trunk/test/AST/ast-dump-openmp-target.c
>> cfe/trunk/test/AST/ast-dump-openmp-task.c
>> cfe/trunk/test/AST/ast-dump-openmp-taskgroup.c
>> cfe/trunk/test/AST/ast-dump-openmp-taskloop-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-taskloop.c
>> cfe/trunk/test/AST/ast-dump-openmp-taskwait.c
>> cfe/trunk/test/AST/ast-dump-openmp-taskyield.c
>> cfe/trunk/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
>> cfe/trunk/test/AST/ast-dump-openmp-teams-distribute-simd.c
>> cfe/trunk/test/AST/ast-dump-openmp-teams-distribute.c
>> cfe/trunk/test/AST/ast-dump-openmp-teams.c
>>
>...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D62638#1525823 , @NoQ wrote:

> Also switched to python2 because it seems to be the default. The differences 
> are minimal.


What about the state of the patch itself? Planned stuff WIP, could I take it to 
SVGify, or?


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

https://reviews.llvm.org/D62638



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

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:151
+super(ExplodedGraph, self).__init__()
+self.nodes = collections.defaultdict(ExplodedNode)
+self.root_id = None

NoQ wrote:
> Charusso wrote:
> > `nodes` -> `graph`
> Mm, why would i have a field called "graph" in an object called "graph".
Because we are constructing the graph object and it would be still much better 
than LLVM's fancy `Graph TheGraph;` logic. I am okay with a bunch of nodes, 
just those are the graph itself.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:191
+.replace('\\>', '>') \
+.rstrip(',')
+logging.debug(raw_json)

NoQ wrote:
> Charusso wrote:
> > I have removed the trailing `,` from the end yesterday so `rstrip(',')` is 
> > not needed.
> > 
> > It would be cool to name the `result[]`, like: `pred_id = result[1]` and 
> > `succ_id = result[2]` or current/previous ID, or something like that.
> > 
> > I also would put the regexes into a function and you could write:
> > `pred_id, succ_id = parse_edge()` or something more simpler.
> > 
> > What is `result[0]` btw? That confused me a little-bit.
> > `pred_id, succ_id = parse_edge()`
> 
> Not really, i'd also have to return whether the match was successful, which 
> kinda makes it as bad as before.
> 
> > What is `result[0]` btw? That confused me a little-bit.
> 
> That's the entire match. 
> https://docs.python.org/3/library/re.html#match-objects
I see, just I am that newbie. Thanks!



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:237
+if p.sloc is not None:
+self._dump(''
+   '%s:%s:%s:'

NoQ wrote:
> Charusso wrote:
> > I think tables are left aligned by default, so you could remove your left 
> > alignments.
> Unfortunately in GraphViz they're centered by default.
Well, that is unfortunate.


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

https://reviews.llvm.org/D62638



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

gribozavr wrote:
> Eugene.Zelenko wrote:
> > gribozavr wrote:
> > > Eugene.Zelenko wrote:
> > > > gribozavr wrote:
> > > > > Eugene.Zelenko wrote:
> > > > > > Return type is not mentioned explicitly, so auto should not be used.
> > > > > An explicit type is not needed for readability here.  The rule is to 
> > > > > use auto when it improves readability, not when the type is not 
> > > > > spelled in immediate vicinity.
> > > > I think it's reasonable to follow modernize-use-auto.
> > > modernize-use-auto is only a heuristic.
> > But set of processed situations are very reasonable.
> In abstract it might sound reasonable.  In practice it is still a heuristic 
> and not a law.
I think it's reasonable to keep in memory that not everybody keeps 
functions/methods' return types in memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:151
+super(ExplodedGraph, self).__init__()
+self.nodes = collections.defaultdict(ExplodedNode)
+self.root_id = None

Charusso wrote:
> `nodes` -> `graph`
Mm, why would i have a field called "graph" in an object called "graph".



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:191
+.replace('\\>', '>') \
+.rstrip(',')
+logging.debug(raw_json)

Charusso wrote:
> I have removed the trailing `,` from the end yesterday so `rstrip(',')` is 
> not needed.
> 
> It would be cool to name the `result[]`, like: `pred_id = result[1]` and 
> `succ_id = result[2]` or current/previous ID, or something like that.
> 
> I also would put the regexes into a function and you could write:
> `pred_id, succ_id = parse_edge()` or something more simpler.
> 
> What is `result[0]` btw? That confused me a little-bit.
> `pred_id, succ_id = parse_edge()`

Not really, i'd also have to return whether the match was successful, which 
kinda makes it as bad as before.

> What is `result[0]` btw? That confused me a little-bit.

That's the entire match. https://docs.python.org/3/library/re.html#match-objects



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:233
+else:
+color = 'cyan3'
+

Charusso wrote:
> It would be cool to put it into a "switch-statement" 
> (https://stackoverflow.com/questions/60208/replacements-for-switch-statement-in-python).
I don't want to list the same color many times for each key that maps into it.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:237
+if p.sloc is not None:
+self._dump(''
+   '%s:%s:%s:'

Charusso wrote:
> I think tables are left aligned by default, so you could remove your left 
> alignments.
Unfortunately in GraphViz they're centered by default.


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

https://reviews.llvm.org/D62638



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

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Also switched to python2 because it seems to be the default. The differences 
are minimal.


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

https://reviews.llvm.org/D62638



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

Eugene.Zelenko wrote:
> gribozavr wrote:
> > Eugene.Zelenko wrote:
> > > gribozavr wrote:
> > > > Eugene.Zelenko wrote:
> > > > > Return type is not mentioned explicitly, so auto should not be used.
> > > > An explicit type is not needed for readability here.  The rule is to 
> > > > use auto when it improves readability, not when the type is not spelled 
> > > > in immediate vicinity.
> > > I think it's reasonable to follow modernize-use-auto.
> > modernize-use-auto is only a heuristic.
> But set of processed situations are very reasonable.
In abstract it might sound reasonable.  In practice it is still a heuristic and 
not a law.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 202502.
NoQ marked 12 inline comments as done.
NoQ added a comment.

Fxd!


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

https://reviews.llvm.org/D62638

Files:
  clang/test/Analysis/exploded-graph-rewriter/edge.dot
  clang/test/Analysis/exploded-graph-rewriter/empty.dot
  clang/test/Analysis/exploded-graph-rewriter/environment.dot
  clang/test/Analysis/exploded-graph-rewriter/lit.local.cfg
  clang/test/Analysis/exploded-graph-rewriter/program_points.dot
  clang/test/Analysis/exploded-graph-rewriter/store.dot
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- /dev/null
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -0,0 +1,397 @@
+#!/usr/bin/env python
+
+from __future__ import print_function
+
+import argparse
+import collections
+import json
+import logging
+import re
+
+
+# A deserialized source location.
+class SourceLocation(object):
+def __init__(self, json_loc):
+super(SourceLocation, self).__init__()
+self.line = json_loc['line']
+self.col = json_loc['column']
+self.filename = json_loc['filename'] \
+if 'filename' in json_loc else '(main file)'
+
+
+# A deserialized program point.
+class ProgramPoint(object):
+def __init__(self, json_pp):
+super(ProgramPoint, self).__init__()
+self.kind = json_pp['kind']
+self.tag = json_pp['tag']
+if self.kind == 'Edge':
+self.src_id = json_pp['src_id']
+self.dst_id = json_pp['dst_id']
+elif self.kind == 'Statement':
+self.stmt_kind = json_pp['stmt_kind']
+self.pointer = json_pp['pointer']
+self.pretty = json_pp['pretty']
+self.loc = SourceLocation(json_pp['location']) \
+if json_pp['location'] is not None else None
+elif self.kind == 'BlockEntrance':
+self.block_id = json_pp['block_id']
+
+
+# A value of a single expression in a deserialized Environment.
+class EnvironmentBinding(object):
+def __init__(self, json_eb):
+super(EnvironmentBinding, self).__init__()
+self.stmt_id = json_eb['stmt_id']
+self.pretty = json_eb['pretty']
+self.value = json_eb['value']
+
+
+# Deserialized description of a location context.
+class LocationContext(object):
+def __init__(self, json_frame):
+super(LocationContext, self).__init__()
+self.lctx_id = json_frame['lctx_id']
+self.caption = json_frame['location_context']
+self.decl = json_frame['calling']
+self.line = json_frame['call_line']
+
+
+# A group of deserialized Environment bindings that correspond to a specific
+# location context.
+class EnvironmentFrame(object):
+def __init__(self, json_frame):
+super(EnvironmentFrame, self).__init__()
+self.location_context = LocationContext(json_frame)
+self.bindings = [EnvironmentBinding(b) for b in json_frame['items']] \
+if json_frame['items'] is not None else []
+
+
+# A deserialized Environment.
+class Environment(object):
+def __init__(self, json_e):
+super(Environment, self).__init__()
+self.frames = [EnvironmentFrame(f) for f in json_e]
+
+
+# A single binding in a deserialized RegionStore cluster.
+class StoreBinding(object):
+def __init__(self, json_sb):
+super(StoreBinding, self).__init__()
+self.kind = json_sb['kind']
+self.offset = json_sb['offset']
+self.value = json_sb['value']
+
+
+# A single cluster of the deserialized RegionStore.
+class StoreCluster(object):
+def __init__(self, json_sc):
+super(StoreCluster, self).__init__()
+self.base_region = json_sc['cluster']
+self.bindings = [StoreBinding(b) for b in json_sc['items']]
+
+
+# A deserialized RegionStore.
+class Store(object):
+def __init__(self, json_s):
+super(Store, self).__init__()
+self.clusters = [StoreCluster(c) for c in json_s]
+
+
+# A deserialized program state.
+class ProgramState(object):
+def __init__(self, state_id, json_ps):
+super(ProgramState, self).__init__()
+logging.debug('Adding ProgramState ' + str(state_id))
+
+self.state_id = state_id
+self.store = Store(json_ps['store']) \
+if json_ps['store'] is not None else None
+self.environment = Environment(json_ps['environment']) \
+if json_ps['environment'] is not None else None
+# TODO: Objects under construction.
+# TODO: Constraint ranges.
+# TODO: Dynamic types of objects.
+# TODO: Checker messages.
+
+
+# A deserialized exploded graph node. Has a default constructor because it
+# may be referenced as part of an edge before its contents are deserialized,
+# and in this moment we already need a room for predecessors and successors.
+class 

[PATCH] D62754: [analyzer] Fix JSON dumps for location contexts.

[Csaba Dabis via Phabricator via cfe-commits]

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

That was my feeling as well, but it has been defined like that. Thanks for the 
clarification!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62754



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

[PATCH] D62754: [analyzer] Fix JSON dumps for location contexts.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

`lctx_id` is a property of a location context, not of an item within it. It's 
useful to know the id even when there are no items in the context.


Repository:
  rC Clang

https://reviews.llvm.org/D62754

Files:
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c


Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -30,8 +30,8 @@
 // CHECK-NEXT: ]}
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "environment": [
-// CHECK-NEXT: { "location_context": "#0 Call", "calling": "foo", 
"call_line": null, "items": [
-// CHECK-NEXT:   { "lctx_id": 1, "stmt_id": {{[0-9]+}}, "pretty": 
"clang_analyzer_printState", "value": "{clang_analyzer_printState}" }
+// CHECK-NEXT: { "lctx_id": 1, "location_context": "#0 Call", "calling": 
"foo", "call_line": null, "items": [
+// CHECK-NEXT:   { "stmt_id": {{[0-9]+}}, "pretty": 
"clang_analyzer_printState", "value": "{clang_analyzer_printState}" }
 // CHECK-NEXT: ]}
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "constraints": [
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -18,9 +18,9 @@
   new S;
 }
 
-// CHECK: \"location_context\": \"#0 Call\", \"calling\": \"foo\", 
\"call_line\": null, \"items\": 
[\l\{ \"lctx_id\": 1, 
\"stmt_id\": {{[0-9]+}}, \"kind\": \"construct into local variable\", 
\"argument_index\": null, \"pretty\": \"T t;\", \"value\": \"\"
+// CHECK: \"location_context\": \"#0 Call\", \"calling\": \"foo\", 
\"call_line\": null, \"items\": 
[\l\{ \"stmt_id\": {{[0-9]+}}, 
\"kind\": \"construct into local variable\", \"argument_index\": null, 
\"pretty\": \"T t;\", \"value\": \"\"
 
-// CHECK: \"location_context\": \"#0 Call\", \"calling\": \"T::T\", 
\"call_line\": \"16\", \"items\": 
[\l\{ \"lctx_id\": 2, 
\"init_id\": {{[0-9]+}}, \"kind\": \"construct into member variable\", 
\"argument_index\": null, \"pretty\": \"s\", \"value\": \"\>s\"
+// CHECK: \"location_context\": \"#0 Call\", \"calling\": \"T::T\", 
\"call_line\": \"16\", \"items\": 
[\l\{ \"init_id\": {{[0-9]+}}, 
\"kind\": \"construct into member variable\", \"argument_index\": null, 
\"pretty\": \"s\", \"value\": \"\>s\"
 
 // CHECK: \"cluster\": \"t\", \"items\": 
[\l\{ \"kind\": \"Default\", 
\"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -149,9 +149,6 @@
 if (!S)
   I = getItem().getCXXCtorInitializer();
 
-// IDs
-Out << "\"lctx_id\": " << getLocationContext()->getID() << ", ";
-
 if (S)
   Out << "\"stmt_id\": " << S->getID(getASTContext());
 else
Index: clang/lib/StaticAnalyzer/Core/Environment.cpp
===
--- clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -261,8 +261,7 @@
 
   const Stmt *S = I->first.getStmt();
   Indent(Out, InnerSpace, IsDot)
-  << "{ \"lctx_id\": " << LC->getID()
-  << ", \"stmt_id\": " << S->getID(Ctx) << ", \"pretty\": ";
+  << "{ \"stmt_id\": " << S->getID(Ctx) << ", \"pretty\": ";
   S->printJson(Out, nullptr, PP, /*AddQuotes=*/true);
 
   Out << ", \"value\": ";
Index: clang/lib/Analysis/AnalysisDeclContext.cpp
===
--- clang/lib/Analysis/AnalysisDeclContext.cpp
+++ clang/lib/Analysis/AnalysisDeclContext.cpp
@@ -527,7 +527,8 @@
 
   unsigned Frame = 0;
   for (const LocationContext *LCtx = this; LCtx; LCtx = LCtx->getParent()) {
-Indent(Out, Space, IsDot) << "{ \"location_context\": \"";
+Indent(Out, Space, IsDot)
+<< "{ \"lctx_id\": " << LCtx->getID() << ", \"location_context\": \"";
 switch (LCtx->getKind()) {
 case StackFrame:
   Out << '#' << Frame << " Call\", \"calling\": \"";
@@ -541,7 +542,7 @@
   if (const Stmt *S = cast(LCtx)->getCallSite()) {
 Out << '\"';
 printLocation(Out, SM, S->getBeginLoc());
-   Out << '\"';
+Out << '\"';
   } else {
 Out << "null";
   }


Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ 

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

gribozavr wrote:
> Eugene.Zelenko wrote:
> > gribozavr wrote:
> > > Eugene.Zelenko wrote:
> > > > Return type is not mentioned explicitly, so auto should not be used.
> > > An explicit type is not needed for readability here.  The rule is to use 
> > > auto when it improves readability, not when the type is not spelled in 
> > > immediate vicinity.
> > I think it's reasonable to follow modernize-use-auto.
> modernize-use-auto is only a heuristic.
But set of processed situations are very reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:54
+
+void e() {
+  int pipefd[2];

srhines wrote:
> gribozavr wrote:
> > How is `e` different from `a`?
> `e` uses O_CLOEXEC properly with `pipe2()` and makes sure that we don't issue 
> additional diagnostics/fixes that are unnecessary.
Thanks -- I see it now.  I think a descriptive test name would help here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049



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

[PATCH] D62752: Move VarBypassDector.h to include/clang/CodeGen

[Zachary Henkel via Phabricator via cfe-commits]

zahen created this revision.
zahen added a reviewer: chandlerc.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Allow the VarBypassDetector class to be consumed by external tools by exposing 
its header from a standard include path.

If accepted, I will need a user with check-in permission to apply the patch.


Repository:
  rC Clang

https://reviews.llvm.org/D62752

Files:
  clang/include/clang/CodeGen/VarBypassDetector.h
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/VarBypassDetector.cpp
  clang/lib/CodeGen/VarBypassDetector.h


Index: clang/lib/CodeGen/VarBypassDetector.cpp
===
--- clang/lib/CodeGen/VarBypassDetector.cpp
+++ clang/lib/CodeGen/VarBypassDetector.cpp
@@ -6,7 +6,7 @@
 //
 
//===--===//
 
-#include "VarBypassDetector.h"
+#include "clang/CodeGen/VarBypassDetector.h"
 
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -20,7 +20,6 @@
 #include "CodeGenModule.h"
 #include "CodeGenPGO.h"
 #include "EHScopeStack.h"
-#include "VarBypassDetector.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CurrentSourceLocExprScope.h"
 #include "clang/AST/ExprCXX.h"
@@ -32,6 +31,7 @@
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/CodeGen/VarBypassDetector.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
Index: clang/include/clang/CodeGen/VarBypassDetector.h
===
--- clang/include/clang/CodeGen/VarBypassDetector.h
+++ clang/include/clang/CodeGen/VarBypassDetector.h
@@ -11,8 +11,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
-#define LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
+#ifndef LLVM_CLANG_CODEGEN_VARBYPASSDETECTOR_H
+#define LLVM_CLANG_CODEGEN_VARBYPASSDETECTOR_H
 
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/DenseMap.h"


Index: clang/lib/CodeGen/VarBypassDetector.cpp
===
--- clang/lib/CodeGen/VarBypassDetector.cpp
+++ clang/lib/CodeGen/VarBypassDetector.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "VarBypassDetector.h"
+#include "clang/CodeGen/VarBypassDetector.h"
 
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -20,7 +20,6 @@
 #include "CodeGenModule.h"
 #include "CodeGenPGO.h"
 #include "EHScopeStack.h"
-#include "VarBypassDetector.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CurrentSourceLocExprScope.h"
 #include "clang/AST/ExprCXX.h"
@@ -32,6 +31,7 @@
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/CodeGen/VarBypassDetector.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
Index: clang/include/clang/CodeGen/VarBypassDetector.h
===
--- clang/include/clang/CodeGen/VarBypassDetector.h
+++ clang/include/clang/CodeGen/VarBypassDetector.h
@@ -11,8 +11,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
-#define LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H
+#ifndef LLVM_CLANG_CODEGEN_VARBYPASSDETECTOR_H
+#define LLVM_CLANG_CODEGEN_VARBYPASSDETECTOR_H
 
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/DenseMap.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362266: Clarify when fix-it hints on warnings are 
appropriate (authored by aaronpuchert, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62470

Files:
  cfe/trunk/docs/InternalsManual.rst


Index: cfe/trunk/docs/InternalsManual.rst
===
--- cfe/trunk/docs/InternalsManual.rst
+++ cfe/trunk/docs/InternalsManual.rst
@@ -423,6 +423,9 @@
   driver, they should only be used when it's very likely they match the user's
   intent.
 * Clang must recover from errors as if the fix-it had been applied.
+* Fix-it hints on a warning must not change the meaning of the code.
+  However, a hint may clarify the meaning as intentional, for example by adding
+  parentheses when the precedence of operators isn't obvious.
 
 If a fix-it can't obey these rules, put the fix-it on a note.  Fix-its on notes
 are not applied automatically.


Index: cfe/trunk/docs/InternalsManual.rst
===
--- cfe/trunk/docs/InternalsManual.rst
+++ cfe/trunk/docs/InternalsManual.rst
@@ -423,6 +423,9 @@
   driver, they should only be used when it's very likely they match the user's
   intent.
 * Clang must recover from errors as if the fix-it had been applied.
+* Fix-it hints on a warning must not change the meaning of the code.
+  However, a hint may clarify the meaning as intentional, for example by adding
+  parentheses when the precedence of operators isn't obvious.
 
 If a fix-it can't obey these rules, put the fix-it on a note.  Fix-its on notes
 are not applied automatically.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r362266 - Clarify when fix-it hints on warnings are appropriate

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Fri May 31 14:27:39 2019
New Revision: 362266

URL: http://llvm.org/viewvc/llvm-project?rev=362266=rev
Log:
Clarify when fix-it hints on warnings are appropriate

Summary:
This is not a change in the rules, it's meant as a clarification about
warnings. Since the recovery from warnings is a no-op, the fix-it hints
on warnings shouldn't change anything. Anything that doesn't just
suppress the warning and changes the meaning of the code (even if it's
for the better) should be on an additional note.

Reviewers: rsmith, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: cfe-commits, thakis

Tags: #clang

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

Modified:
cfe/trunk/docs/InternalsManual.rst

Modified: cfe/trunk/docs/InternalsManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/InternalsManual.rst?rev=362266=362265=362266=diff
==
--- cfe/trunk/docs/InternalsManual.rst (original)
+++ cfe/trunk/docs/InternalsManual.rst Fri May 31 14:27:39 2019
@@ -423,6 +423,9 @@ Fix-it hints on errors and warnings need
   driver, they should only be used when it's very likely they match the user's
   intent.
 * Clang must recover from errors as if the fix-it had been applied.
+* Fix-it hints on a warning must not change the meaning of the code.
+  However, a hint may clarify the meaning as intentional, for example by adding
+  parentheses when the precedence of operators isn't obvious.
 
 If a fix-it can't obey these rules, put the fix-it on a note.  Fix-its on notes
 are not applied automatically.


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

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 202483.
aaronpuchert added a comment.

Add missing CHECK-NOTs for -Wmissing-prototypes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c
  test/Sema/warn-missing-variable-declarations.c
  test/SemaCXX/warn-missing-prototypes.cpp
  test/SemaCXX/warn-missing-variable-declarations.cpp
  test/SemaOpenCL/warn-missing-prototypes.cl

Index: test/SemaOpenCL/warn-missing-prototypes.cl
===
--- test/SemaOpenCL/warn-missing-prototypes.cl
+++ test/SemaOpenCL/warn-missing-prototypes.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 
 // Don't warn about kernel functions.
 kernel void g() { }
Index: test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- test/SemaCXX/warn-missing-variable-declarations.cpp
+++ test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-variable-declarations -std=c++17 %s
 
 // Variable declarations that should trigger a warning.
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
+
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 namespace x {
   int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+  // expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 }
 
 // Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@
 constexpr int constexpr_var = 0;
 inline constexpr int inline_constexpr_var = 0;
 extern const int extern_const_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 template int var_template = 0;
 template constexpr int const_var_template = 0;
@@ -69,7 +75,9 @@
 template int var_template;
 extern template int var_template;
 template<> int var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 // FIXME: We give this specialization internal linkage rather than inheriting
 // the linkage from the template! We should not warn here.
 template<> int static_var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
Index: test/SemaCXX/warn-missing-prototypes.cpp
===
--- test/SemaCXX/warn-missing-prototypes.cpp
+++ test/SemaCXX/warn-missing-prototypes.cpp
@@ -1,9 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
 
 namespace NS {
   void f() { } // expected-warning {{no previous prototype for function 'f'}}
+  // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 }
 
 namespace {
@@ -32,3 +36,7 @@
 
 // Don't warn on explicitly deleted functions.
 void j() = delete;
+
+extern void k() {} // expected-warning {{no previous prototype for function 'k'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:{{.*}}-[[@LINE-2]]:{{.*}}}:"{{.*}}"
Index: test/Sema/warn-missing-variable-declarations.c
===
--- 

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 202480.
aaronpuchert added a comment.

Remove other change from this one.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c
  test/Sema/warn-missing-variable-declarations.c
  test/SemaCXX/warn-missing-prototypes.cpp
  test/SemaCXX/warn-missing-variable-declarations.cpp
  test/SemaOpenCL/warn-missing-prototypes.cl

Index: test/SemaOpenCL/warn-missing-prototypes.cl
===
--- test/SemaOpenCL/warn-missing-prototypes.cl
+++ test/SemaOpenCL/warn-missing-prototypes.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 
 // Don't warn about kernel functions.
 kernel void g() { }
Index: test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- test/SemaCXX/warn-missing-variable-declarations.cpp
+++ test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-variable-declarations -std=c++17 %s
 
 // Variable declarations that should trigger a warning.
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
+
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 namespace x {
   int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+  // expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 }
 
 // Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@
 constexpr int constexpr_var = 0;
 inline constexpr int inline_constexpr_var = 0;
 extern const int extern_const_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 template int var_template = 0;
 template constexpr int const_var_template = 0;
@@ -69,7 +75,9 @@
 template int var_template;
 extern template int var_template;
 template<> int var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 // FIXME: We give this specialization internal linkage rather than inheriting
 // the linkage from the template! We should not warn here.
 template<> int static_var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
Index: test/SemaCXX/warn-missing-prototypes.cpp
===
--- test/SemaCXX/warn-missing-prototypes.cpp
+++ test/SemaCXX/warn-missing-prototypes.cpp
@@ -1,9 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
 
 namespace NS {
   void f() { } // expected-warning {{no previous prototype for function 'f'}}
+  // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 }
 
 namespace {
@@ -32,3 +36,6 @@
 
 // Don't warn on explicitly deleted functions.
 void j() = delete;
+
+extern void k() {} // expected-warning {{no previous prototype for function 'k'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
Index: test/Sema/warn-missing-variable-declarations.c
===
--- test/Sema/warn-missing-variable-declarations.c
+++ 

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 202479.
aaronpuchert added a comment.
This revision is now accepted and ready to land.

Show note suggesting to add `static`, move fix-it to note for functions, remove 
fix-it for variables. Show note even for `extern` function definitions when we 
can't build a proper fix-it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c
  test/Sema/warn-missing-variable-declarations.c
  test/SemaCXX/warn-missing-prototypes.cpp
  test/SemaCXX/warn-missing-variable-declarations.cpp
  test/SemaOpenCL/warn-missing-prototypes.cl

Index: test/SemaOpenCL/warn-missing-prototypes.cl
===
--- test/SemaOpenCL/warn-missing-prototypes.cl
+++ test/SemaOpenCL/warn-missing-prototypes.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 
 // Don't warn about kernel functions.
 kernel void g() { }
Index: test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- test/SemaCXX/warn-missing-variable-declarations.cpp
+++ test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-variable-declarations -std=c++17 %s
 
 // Variable declarations that should trigger a warning.
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
+
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 namespace x {
   int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+  // expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 }
 
 // Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@
 constexpr int constexpr_var = 0;
 inline constexpr int inline_constexpr_var = 0;
 extern const int extern_const_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 template int var_template = 0;
 template constexpr int const_var_template = 0;
@@ -69,7 +75,9 @@
 template int var_template;
 extern template int var_template;
 template<> int var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 // FIXME: We give this specialization internal linkage rather than inheriting
 // the linkage from the template! We should not warn here.
 template<> int static_var_template; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
Index: test/SemaCXX/warn-missing-prototypes.cpp
===
--- test/SemaCXX/warn-missing-prototypes.cpp
+++ test/SemaCXX/warn-missing-prototypes.cpp
@@ -1,9 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
 
 namespace NS {
   void f() { } // expected-warning {{no previous prototype for function 'f'}}
+  // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 }
 
 namespace {
@@ -32,3 +36,6 @@
 
 // Don't warn on explicitly deleted functions.
 void j() = delete;
+
+extern void k() {} // expected-warning {{no previous prototype for function 'k'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
Index: 

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

I've factored out some changes into D62750  
and will rebase this on top if it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:7885
+// By default, restrict the maximum size to 256.
+F->addFnAttr("amdgpu-flat-work-group-size", "128,256");
   }

arsenm wrote:
> b-sumner wrote:
> > Theoretically, shouldn't the minimum be 1?
> That's what I thought, but the backend is defaulting to 2 * wave size now
I don't get it. This attribute indicates the possible workgroup size range this 
kernel may be run with, right? It only depends on how user execute the kernel. 
How is it related to backend defaults?


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

https://reviews.llvm.org/D62739



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

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There was a search for non-prototype declarations for the function, but
we only showed the results for zero-parameter functions. Now we show the
note for functions with parameters as well, but we omit the fix-it hint
suggesting to add `void`.


Repository:
  rC Clang

https://reviews.llvm.org/D62750

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c

Index: test/Sema/warn-missing-prototypes.c
===
--- test/Sema/warn-missing-prototypes.c
+++ test/Sema/warn-missing-prototypes.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wmissing-prototypes -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
-int f();
+int f(); // expected-note{{this declaration is not a prototype; add parameter declarations to make it one}}
 
 int f(int x) { return x; } // expected-warning{{no previous prototype for function 'f'}}
 
@@ -15,7 +15,7 @@
 
 void test(void);
 
-int h3();
+int h3(); // expected-note{{this declaration is not a prototype; add parameter declarations to make it one}}
 int h4(int);
 int h4();
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12747,8 +12747,9 @@
   Consumer.HandleInlineFunctionDefinition(D);
 }
 
-static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
- const FunctionDecl*& PossibleZeroParamPrototype) {
+static bool
+ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
+const FunctionDecl *) {
   // Don't warn about invalid declarations.
   if (FD->isInvalidDecl())
 return false;
@@ -12785,7 +12786,6 @@
   if (FD->isDeleted())
 return false;
 
-  bool MissingPrototype = true;
   for (const FunctionDecl *Prev = FD->getPreviousDecl();
Prev; Prev = Prev->getPreviousDecl()) {
 // Ignore any declarations that occur in function or method
@@ -12793,13 +12793,11 @@
 if (Prev->getLexicalDeclContext()->isFunctionOrMethod())
   continue;
 
-MissingPrototype = !Prev->getType()->isFunctionProtoType();
-if (FD->getNumParams() == 0)
-  PossibleZeroParamPrototype = Prev;
-break;
+PossiblePrototype = Prev;
+return Prev->getType()->isFunctionNoProtoType();
   }
 
-  return MissingPrototype;
+  return true;
 }
 
 void
@@ -13319,21 +13317,22 @@
 //   prototype declaration. This warning is issued even if the
 //   definition itself provides a prototype. The aim is to detect
 //   global functions that fail to be declared in header files.
-const FunctionDecl *PossibleZeroParamPrototype = nullptr;
-if (ShouldWarnAboutMissingPrototype(FD, PossibleZeroParamPrototype)) {
+const FunctionDecl *PossiblePrototype = nullptr;
+if (ShouldWarnAboutMissingPrototype(FD, PossiblePrototype)) {
   Diag(FD->getLocation(), diag::warn_missing_prototype) << FD;
 
-  if (PossibleZeroParamPrototype) {
+  if (PossiblePrototype) {
 // We found a declaration that is not a prototype,
 // but that could be a zero-parameter prototype
-if (TypeSourceInfo *TI =
-PossibleZeroParamPrototype->getTypeSourceInfo()) {
+if (TypeSourceInfo *TI = PossiblePrototype->getTypeSourceInfo()) {
   TypeLoc TL = TI->getTypeLoc();
   if (FunctionNoProtoTypeLoc FTL = TL.getAs())
-Diag(PossibleZeroParamPrototype->getLocation(),
+Diag(PossiblePrototype->getLocation(),
  diag::note_declaration_not_a_prototype)
-<< PossibleZeroParamPrototype
-<< FixItHint::CreateInsertion(FTL.getRParenLoc(), "void");
+<< (FD->getNumParams() != 0)
+<< (FD->getNumParams() == 0
+? FixItHint::CreateInsertion(FTL.getRParenLoc(), "void")
+: FixItHint{});
 }
   }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -4652,7 +4652,8 @@
   "no previous prototype for function %0">,
   InGroup>, DefaultIgnore;
 def note_declaration_not_a_prototype : Note<
-  "this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function">;
+  "this declaration is not a prototype; add %select{'void'|parameter declarations}0 "
+  "to make it %select{a prototype for a zero-parameter function|one}0">;
 def warn_strict_prototypes : Warning<
   "this %select{function declaration is not|block declaration is not|"
   "old-style function definition is 

[PATCH] D62746: msabi: Fix exponential mangling time for certain pathological inputs

[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

Thanks for this, I remember a user complained about this a long time ago but we 
never did anything.


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

https://reviews.llvm.org/D62746



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

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:54
+
+void e() {
+  int pipefd[2];

gribozavr wrote:
> How is `e` different from `a`?
`e` uses O_CLOEXEC properly with `pipe2()` and makes sure that we don't issue 
additional diagnostics/fixes that are unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049



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

[PATCH] D62746: msabi: Fix exponential mangling time in certain pathological inputs

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rnk.

Template back references used to be recursively recomputed, add a
memoization cache to cut down on this.

Since there are now two different types of argument maps, rename the
existing TypeBackReferences to FunArgBackReferences, and rename
mangleArgumentType() to mangleFunctionArgumentType().

Fixes PR42091, the input there now takes 50ms instead of 7s to compile.

No intended behavior change.


https://reviews.llvm.org/D62746

Files:
  clang/lib/AST/MicrosoftMangle.cpp

Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -265,7 +265,8 @@
   BackRefVec NameBackReferences;
 
   typedef llvm::DenseMap ArgBackRefMap;
-  ArgBackRefMap TypeBackReferences;
+  ArgBackRefMap FunArgBackReferences;
+  ArgBackRefMap TemplateArgBackReferences;
 
   typedef std::set> PassObjectSizeArgsSet;
   PassObjectSizeArgsSet PassObjectSizeArgs;
@@ -343,7 +344,7 @@
   const TemplateArgumentList );
   void mangleObjCMethodName(const ObjCMethodDecl *MD);
 
-  void mangleArgumentType(QualType T, SourceRange Range);
+  void mangleFunctionArgumentType(QualType T, SourceRange Range);
   void manglePassObjectSizeArg(const PassObjectSizeAttr *POSA);
 
   bool isArtificialTagType(QualType T) const;
@@ -793,7 +794,7 @@
 // the X part is aliased. However, if you need to mangle
 //   void foo(A::X, A::X),
 // the A::X<> part is not aliased.
-// That said, from the mangler's perspective we have a structure like this:
+// That is, from the mangler's perspective we have a structure like this:
 //   namespace[s] -> type[ -> template-parameters]
 // but from the Clang perspective we have
 //   type [ -> template-parameters]
@@ -803,12 +804,30 @@
 // the mangled type name as a key to check the mangling of different types
 // for aliasing.
 
-llvm::SmallString<64> TemplateMangling;
-llvm::raw_svector_ostream Stream(TemplateMangling);
-MicrosoftCXXNameMangler Extra(Context, Stream);
-Extra.mangleTemplateInstantiationName(TD, *TemplateArgs);
-
-mangleSourceName(TemplateMangling);
+// It's important to key cache reads off ND, not TD -- the same TD can
+// be used with different TemplateArgs, but ND uniquely identifies
+// TD / TemplateArg pairs.
+ArgBackRefMap::iterator Found = TemplateArgBackReferences.find(ND);
+if (Found == TemplateArgBackReferences.end()) {
+  // Mangle full template name into temporary buffer.
+  llvm::SmallString<64> TemplateMangling;
+  llvm::raw_svector_ostream Stream(TemplateMangling);
+  MicrosoftCXXNameMangler Extra(Context, Stream);
+  Extra.mangleTemplateInstantiationName(TD, *TemplateArgs);
+
+  // Use the string backref vector to possibly get a back reference.
+  mangleSourceName(TemplateMangling);
+
+  // Memoize back reference for this type.
+  BackRefVec::iterator StringFound =
+  llvm::find(NameBackReferences, TemplateMangling);
+  if (StringFound != NameBackReferences.end()) {
+TemplateArgBackReferences[ND] =
+StringFound - NameBackReferences.begin();
+  }
+} else {
+  Out << Found->second;
+}
 return;
   }
 
@@ -1282,11 +1301,13 @@
   // Always start with the unqualified name.
 
   // Templates have their own context for back references.
-  ArgBackRefMap OuterArgsContext;
+  ArgBackRefMap OuterFunArgsContext;
+  ArgBackRefMap OuterTemplateArgsContext;
   BackRefVec OuterTemplateContext;
   PassObjectSizeArgsSet OuterPassObjectSizeArgs;
   NameBackReferences.swap(OuterTemplateContext);
-  TypeBackReferences.swap(OuterArgsContext);
+  FunArgBackReferences.swap(OuterFunArgsContext);
+  TemplateArgBackReferences.swap(OuterTemplateArgsContext);
   PassObjectSizeArgs.swap(OuterPassObjectSizeArgs);
 
   mangleUnscopedTemplateName(TD);
@@ -1294,7 +1315,8 @@
 
   // Restore the previous back reference contexts.
   NameBackReferences.swap(OuterTemplateContext);
-  TypeBackReferences.swap(OuterArgsContext);
+  FunArgBackReferences.swap(OuterFunArgsContext);
+  TemplateArgBackReferences.swap(OuterTemplateArgsContext);
   PassObjectSizeArgs.swap(OuterPassObjectSizeArgs);
 }
 
@@ -1699,8 +1721,8 @@
   }
 }
 
-void MicrosoftCXXNameMangler::mangleArgumentType(QualType T,
- SourceRange Range) {
+void MicrosoftCXXNameMangler::mangleFunctionArgumentType(QualType T,
+ SourceRange Range) {
   // MSVC will backreference two canonically equivalent types that have slightly
   // different manglings when mangled alone.
 
@@ -1730,9 +1752,9 @@
 TypePtr = T.getCanonicalType().getAsOpaquePtr();
   }
 
-  ArgBackRefMap::iterator Found = TypeBackReferences.find(TypePtr);
+  ArgBackRefMap::iterator Found = FunArgBackReferences.find(TypePtr);
 
-  if 

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:17
+  pipe(pipefd);
+  // CHECK-MESSAGES-NOT: warning:
+}

hokein wrote:
> nit: no need to do it explicitly, if a warning is shown unexpectedly, the 
> test will fail.
I somehow never realized this (and there seem to be several places that also 
don't realize it). Thanks for letting us know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

Eugene.Zelenko wrote:
> gribozavr wrote:
> > Eugene.Zelenko wrote:
> > > Return type is not mentioned explicitly, so auto should not be used.
> > An explicit type is not needed for readability here.  The rule is to use 
> > auto when it improves readability, not when the type is not spelled in 
> > immediate vicinity.
> I think it's reasonable to follow modernize-use-auto.
modernize-use-auto is only a heuristic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:317
+
+  if (pipe(InotifyPollingStopperFDs) == -1)
+return nullptr;

Use pipe2() with O_CLOEXEC, to avoid leaking the file descriptors to child 
processes?


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

https://reviews.llvm.org/D58418



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40
+// FDRead.
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {

jkorous wrote:
> gribozavr wrote:
> > gribozavr wrote:
> > > Three slashes for doc comments?
> > Don't write what something is used for currently, such comments go out of 
> > date quickly.  (Just delete the last sentence.)
> Sure, no problem.
> 
> I naturally tend to err on the side of over-commenting as I think I'd 
> appreciate it myself if I had to understand the code without prior knowledge 
> - not saying it's intentional or that I have a strong reason to do that 
> though. You seem to have a strong preference for not having out-of-date 
> comments with low-ish information value. Just out of curiosity - is it based 
> on any particular reason or experience?
> is it based on any particular reason or experience?

Yes, primarily working on the Swift compiler and the standard library, when 
everything was changing very quickly.  My current work also confirms the same 
-- a lot of time when I see a comment about the "current usage" or other 
incidental information that is not the contract of the API, it tends to be 
outdated.  Approximately nobody will change such a comment when another user is 
added ("I'm just reusing the code..."), even when the quoted user already 
became non-representative of the usage pattern, or the usage pattern has 
changed.  However, when changing the API contract people typically do change 
the comment.

It also makes sense to me in abstract: reading that X happens to be used for Y 
does not necessarily help understand X better -- it is only a cross-reference 
that I could find myself with an IDE command; I still need to understand the 
design of Y and the interaction with X, and then using my past experience infer 
what X was intended to be.

Saying that X is intended to be used only by Y is a different story of course, 
that's design documentation.

Providing an example of usage is also fine, but it should be phrased as an 
example that can't become stale.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:52
+close(FDWrite);
+close(FDRead);
+  }

jkorous wrote:
> gribozavr wrote:
> > Since it closes the file descriptors in the destructor, I feel like it 
> > should also be responsible for calling `pipe` in the constructor.
> I know what you mean - my "oop feel" was telling me it's wrong too. It's not 
> just the pipe in this class but also the inotify descriptor in the watcher 
> class.
> 
> The problem is that the only reasonable way how to communicate failures from 
> constructors that I am aware of are exceptions which we don't use in llvm. 
> That's why I moved most of the stuff that can fail even before any work is 
> actually started to the factory method (with the exception of epoll file 
> descriptor as that felt like making its scope unnecessarily larger).
> 
> Thinking about it now, I am starting to doubt that it makes life any easier 
> for client code as it still has to cope with failure communicated as 
> WatcherGotInvalidated event via receiver.
> 
> What do you think?
You could add a factory function to SemaphorePipe, but... I feel like trying to 
recover from a failure in the pipe() call is a bit like trying to recover from 
a memory allocation failure.  The process is probably hitting a file descriptor 
limit or something like that, and is likely going to fail anyway.  I'd probably 
trigger a fatal error if pipe() fails.

This class is a lot like pthread_mutex_init -- it can fail "gracefully", but 
there's no way for the caller to recover -- the caller needs a mutex to proceed.


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

https://reviews.llvm.org/D58418



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

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Something I ran into when reviewing https://reviews.llvm.org/D62639 is that on 
AArch64, for varargs functions, we emit floating-point stores when 
noimplicitfloat is specified.  That seems fine for -mno-implicit-float, but 
maybe not for -mgeneral-regs-only?


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

https://reviews.llvm.org/D38479



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Jan Korous via Phabricator via cfe-commits]

jkorous updated this revision to Diff 202467.
jkorous marked 2 inline comments as done.
jkorous added a comment.

Addressed comments.


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

https://reviews.llvm.org/D58418

Files:
  clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
  clang/lib/CMakeLists.txt
  clang/lib/DirectoryWatcher/CMakeLists.txt
  clang/lib/DirectoryWatcher/DirectoryScanner.cpp
  clang/lib/DirectoryWatcher/DirectoryScanner.h
  clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
  clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/DirectoryWatcher/CMakeLists.txt
  clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===
--- /dev/null
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -0,0 +1,426 @@
+//===- unittests/DirectoryWatcher/DirectoryWatcherTest.cpp ===//
+//
+// 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/DirectoryWatcher/DirectoryWatcher.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Mutex.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+
+using namespace llvm;
+using namespace llvm::sys;
+using namespace llvm::sys::fs;
+using namespace clang;
+
+namespace clang {
+static bool operator==(const DirectoryWatcher::Event ,
+   const DirectoryWatcher::Event ) {
+  return lhs.Filename == rhs.Filename &&
+ static_cast(lhs.Kind) == static_cast(rhs.Kind);
+}
+} // namespace clang
+
+namespace {
+
+struct DirectoryWatcherTestFixture {
+  std::string TestRootDir;
+  std::string TestWatchedDir;
+
+  DirectoryWatcherTestFixture() {
+SmallString<128> pathBuf;
+std::error_code UniqDirRes = createUniqueDirectory("dirwatcher", pathBuf);
+assert(!UniqDirRes);
+TestRootDir = pathBuf.str();
+path::append(pathBuf, "watch");
+TestWatchedDir = pathBuf.str();
+std::error_code CreateDirRes = create_directory(TestWatchedDir, false);
+assert(!CreateDirRes);
+  }
+
+  ~DirectoryWatcherTestFixture() { remove_directories(TestRootDir); }
+
+  SmallString<128> getPathInWatched(const std::string ) {
+SmallString<128> pathBuf;
+pathBuf = TestWatchedDir;
+path::append(pathBuf, testFile);
+return pathBuf;
+  }
+
+  void addFile(const std::string ) {
+Expected ft = openNativeFileForWrite(getPathInWatched(testFile),
+ CD_CreateNew, OF_None);
+if (ft) {
+  closeFile(*ft);
+} else {
+  llvm::errs() << llvm::toString(ft.takeError()) << "\n";
+  llvm::errs() << getPathInWatched(testFile) << "\n";
+  llvm_unreachable("Couldn't create test file.");
+}
+  }
+
+  void deleteFile(const std::string ) {
+std::error_code EC =
+remove(getPathInWatched(testFile), /*IgnoreNonExisting=*/false);
+ASSERT_FALSE(EC);
+  }
+};
+
+std::string eventKindToString(const DirectoryWatcher::Event::EventKind K) {
+  switch (K) {
+  case DirectoryWatcher::Event::EventKind::Removed:
+return "Removed";
+  case DirectoryWatcher::Event::EventKind::Modified:
+return "Modified";
+  case DirectoryWatcher::Event::EventKind::WatchedDirRemoved:
+return "WatchedDirRemoved";
+  case DirectoryWatcher::Event::EventKind::WatcherGotInvalidated:
+return "WatcherGotInvalidated";
+  }
+  llvm_unreachable("unknown event kind");
+}
+
+struct VerifyingConsumer {
+  std::vector ExpectedInitial;
+  std::vector ExpectedNonInitial;
+  std::vector OptionalNonInitial;
+  std::vector UnexpectedInitial;
+  std::vector UnexpectedNonInitial;
+  std::mutex Mtx;
+  std::condition_variable ResultIsReady;
+
+  VerifyingConsumer(
+  const std::vector ,
+  const std::vector ,
+  const std::vector  = {})
+  : ExpectedInitial(ExpectedInitial),
+ExpectedNonInitial(ExpectedNonInitial),
+OptionalNonInitial(OptionalNonInitial) {}
+
+  // This method is used by DirectoryWatcher.
+  void consume(DirectoryWatcher::Event E, bool IsInitial) {
+if (IsInitial)
+  consumeInitial(E);
+else
+  consumeNonInitial(E);
+  }
+
+  void consumeInitial(DirectoryWatcher::Event E) {
+std::unique_lock L(Mtx);
+auto It = std::find(ExpectedInitial.begin(), ExpectedInitial.end(), E);
+if (It == ExpectedInitial.end()) {
+  UnexpectedInitial.push_back(E);
+} else {
+  ExpectedInitial.erase(It);
+}
+if (result())
+  ResultIsReady.notify_one();
+  }
+
+  void consumeNonInitial(DirectoryWatcher::Event E) {
+std::unique_lock L(Mtx);
+ 

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Jan Korous via Phabricator via cfe-commits]

jkorous marked 20 inline comments as done.
jkorous added inline comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40
+// FDRead.
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {

gribozavr wrote:
> gribozavr wrote:
> > Three slashes for doc comments?
> Don't write what something is used for currently, such comments go out of 
> date quickly.  (Just delete the last sentence.)
Sure, no problem.

I naturally tend to err on the side of over-commenting as I think I'd 
appreciate it myself if I had to understand the code without prior knowledge - 
not saying it's intentional or that I have a strong reason to do that though. 
You seem to have a strong preference for not having out-of-date comments with 
low-ish information value. Just out of curiosity - is it based on any 
particular reason or experience?



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:52
+close(FDWrite);
+close(FDRead);
+  }

gribozavr wrote:
> Since it closes the file descriptors in the destructor, I feel like it should 
> also be responsible for calling `pipe` in the constructor.
I know what you mean - my "oop feel" was telling me it's wrong too. It's not 
just the pipe in this class but also the inotify descriptor in the watcher 
class.

The problem is that the only reasonable way how to communicate failures from 
constructors that I am aware of are exceptions which we don't use in llvm. 
That's why I moved most of the stuff that can fail even before any work is 
actually started to the factory method (with the exception of epoll file 
descriptor as that felt like making its scope unnecessarily larger).

Thinking about it now, I am starting to doubt that it makes life any easier for 
client code as it still has to cope with failure communicated as 
WatcherGotInvalidated event via receiver.

What do you think?



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:153
+  // Not locking - caller has to lock Mtx
+  llvm::Optional Result() const {
+if (ExpectedInitial.empty() && ExpectedNonInitial.empty() &&

gribozavr wrote:
> Please name functions consistently -- there's both `consume()` that starts 
> with lowercase, and `Result()` that starts with uppercase.
> 
> Please refer to the current naming rules in the style guide and apply 
> everywhere in the patch.
> 
> 
Sorry about that. This is definitely my weak point.


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

https://reviews.llvm.org/D58418



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

gribozavr wrote:
> Eugene.Zelenko wrote:
> > Return type is not mentioned explicitly, so auto should not be used.
> An explicit type is not needed for readability here.  The rule is to use auto 
> when it improves readability, not when the type is not spelled in immediate 
> vicinity.
I think it's reasonable to follow modernize-use-auto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr marked an inline comment as done.
gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

Eugene.Zelenko wrote:
> Return type is not mentioned explicitly, so auto should not be used.
An explicit type is not needed for readability here.  The rule is to use auto 
when it improves readability, not when the type is not spelled in immediate 
vicinity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62279: Use LTO capable linker

[Chris Bieneman via Phabricator via cfe-commits]

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

This is good to land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62279



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

[PATCH] D62658: [analyzer] print() JSONify: ExplodedNode revision

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362249: [analyzer] print() JSONify: ExplodedNode revision 
(authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62658?vs=202173=202462#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62658

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp


Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3075,8 +3075,8 @@
 const unsigned int Space = 1;
 ProgramStateRef State = N->getState();
 
-Out << "{ \"node_id\": \"" << (const void *)N
-<< "\", \"state_id\": " << State->getID()
+Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
+<< (const void *)N << "\", \"state_id\": " << State->getID()
 << ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
 << ",\\l";
 
@@ -3094,7 +3094,7 @@
   else
 Out << "null }";
 },
-   // Adds a comma and a new-line between each program point.
+// Adds a comma and a new-line between each program point.
 [&](const ExplodedNode *) { Out << ",\\l"; },
 [&](const ExplodedNode *) { return false; });
 


Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3075,8 +3075,8 @@
 const unsigned int Space = 1;
 ProgramStateRef State = N->getState();
 
-Out << "{ \"node_id\": \"" << (const void *)N
-<< "\", \"state_id\": " << State->getID()
+Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
+<< (const void *)N << "\", \"state_id\": " << State->getID()
 << ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
 << ",\\l";
 
@@ -3094,7 +3094,7 @@
   else
 Out << "null }";
 },
-	// Adds a comma and a new-line between each program point.
+// Adds a comma and a new-line between each program point.
 [&](const ExplodedNode *) { Out << ",\\l"; },
 [&](const ExplodedNode *) { return false; });
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r362249 - [analyzer] print() JSONify: ExplodedNode revision

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Fri May 31 10:54:12 2019
New Revision: 362249

URL: http://llvm.org/viewvc/llvm-project?rev=362249=rev
Log:
[analyzer] print() JSONify: ExplodedNode revision

Summary: Revert node-ID removal.

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Subscribers: szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp,
 cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=362249=362248=362249=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri May 31 10:54:12 2019
@@ -3075,8 +3075,8 @@ struct DOTGraphTraits :
 const unsigned int Space = 1;
 ProgramStateRef State = N->getState();
 
-Out << "{ \"node_id\": \"" << (const void *)N
-<< "\", \"state_id\": " << State->getID()
+Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
+<< (const void *)N << "\", \"state_id\": " << State->getID()
 << ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
 << ",\\l";
 
@@ -3094,7 +3094,7 @@ struct DOTGraphTraits :
   else
 Out << "null }";
 },
-   // Adds a comma and a new-line between each program point.
+// Adds a comma and a new-line between each program point.
 [&](const ExplodedNode *) { Out << ",\\l"; },
 [&](const ExplodedNode *) { return false; });
 


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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Matt Arsenault via Phabricator via cfe-commits]

arsenm marked an inline comment as done.
arsenm added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:7885
+// By default, restrict the maximum size to 256.
+F->addFnAttr("amdgpu-flat-work-group-size", "128,256");
   }

b-sumner wrote:
> Theoretically, shouldn't the minimum be 1?
That's what I thought, but the backend is defaulting to 2 * wave size now


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

https://reviews.llvm.org/D62739



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D53157#1525311 , @kpn wrote:

> Oh, this ticket is not going to die from neglect. It is true that D43515 
>  is a higher priority, but I need this 
> IRBuilder work done as well. My department head wanted it done by the end of 
> _last_ year. It's not going to die.
>
> How about I merge your changes into this ticket and we continue work over 
> here? There is the issue of the documentation that lebedev.ri asked you to 
> write. Can I talk you into putting that together and sending it to me 
> ? I'll work on the documentation that I was asked to 
> write. Between the two of us we should be in pretty good shape. Does that 
> work for you?


Yes I'll do that. Thanks.

> I am still waiting for feedback from an actual consumer of the IRBuilder who 
> will be using this new functionality. If someone clang-side could chime in on 
> this ticket I'd very much appreciate it.

I wrote a clang patch that works with your IRBuilder modifications.  I also 
checked with the Intel fortran team and they think this interface will be 
workable for them too. From Intel perspective it's +1.


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

https://reviews.llvm.org/D53157



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =

Return type is not mentioned explicitly, so auto should not be used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert removed a reviewer: bkramer.
aaronpuchert added a comment.

I discovered an issue with variables, because there can be multiple definitions 
with the same `TypeLoc`:

  extern int a;
  int a, b;

We emit the warning for `b`, but we can't just put static in front of the 
declaration, as that would affect `a` as well. I haven't found a way to 
determine whether a `VarDecl` shares its type with another declaration, 
especially since global declarations don't even belong to a `DeclStmt`. We just 
get

  TranslationUnitDecl 0x1d0b598 <> 
  |-...
  |-VarDecl 0x1d47b60 <:1:1, col:12> col:12 a 'int' extern
  |-VarDecl 0x1d47c18 prev 0x1d47b60  col:5 a 'int'
  `-VarDecl 0x1d47c90  col:8 b 'int'

So I think I have to drop the fix-it hint there, but I can still emit a note.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr requested changes to this revision.
gribozavr added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe2.rst:6
+
+Checks if the required file flag ``O_CLOEXEC`` is present in the argument of
+``pipe2()``. ``pipe2()`` should include ``O_CLOEXEC`` in its type argument to

This checks ensures that pipe2() is called with the O_CLOEXEC flag.  This flag 
helps to avoid accidentally leaking file descriptors to child processes.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe2.rst:17
+
+  // becomes
+

"Suggested replacement"

Also use a separate code block for the replacement.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:48
+  pipe2(pipefd, O_NONBLOCK);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(pipe2(pipefd, O_NONBLOCK));

srhines wrote:
> Much like line 39 (which covers both lines 37 and 38), you can delete this 
> CHECK-MESSAGES-NOT. The one on line 50 will cover both of these.
Not done -- you don't need any CHECK-MESSAGES-NOT at all.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:20
+  pipe2(pipefd, O_NONBLOCK);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'pipe2' should use O_CLOEXEC 
where possible [android-cloexec-pipe2]
+  // CHECK-FIXES: pipe2(pipefd, O_NONBLOCK | O_CLOEXEC);

Same comment about the message as in D61967 -- the message should briefly 
explain why the user should make this change. Users tend to ignore warnings 
they don't understand.

"pipe2 should be called with O_CLOEXEC to avoid leaking file descriptors to 
child processes"



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:27
+
+void f() {
+  int pipefd[2];

Please give the tests informative names instead of `a`, `f`, `g` etc.



Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:54
+
+void e() {
+  int pipefd[2];

How is `e` different from `a`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049



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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:7885
+// By default, restrict the maximum size to 256.
+F->addFnAttr("amdgpu-flat-work-group-size", "128,256");
   }

Theoretically, shouldn't the minimum be 1?


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

https://reviews.llvm.org/D62739



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

r362245 - Replace 'default' in an enum-over-a-switch with the missing list.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May 31 10:00:48 2019
New Revision: 362245

URL: http://llvm.org/viewvc/llvm-project?rev=362245=rev
Log:
Replace 'default' in an enum-over-a-switch with the missing list.

This suppressed the Wswitch warning causing me to miss it and write an
assertion failure.

Modified:
cfe/trunk/lib/Sema/SemaExceptionSpec.cpp

Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=362245=362244=362245=diff
==
--- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Fri May 31 10:00:48 2019
@@ -429,7 +429,11 @@ bool Sema::CheckEquivalentExceptionSpec(
   case EST_NoThrow:
 OS <<"__attribute__((nothrow))";
 break;
-  default:
+  case EST_None:
+  case EST_MSAny:
+  case EST_Unevaluated:
+  case EST_Uninstantiated:
+  case EST_Unparsed:
 llvm_unreachable("This spec type is compatible with none.");
   }
 


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

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:31
+  Result,
+  "prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC",
+  ReplacementText);

hokein wrote:
> the message doesn't seem to explain the reason, especially to the people who 
> are not familiar with the `pipe` API, maybe `prefer pipe2() to pipe() to 
> avoid file descriptor leakage` is clearer?
> 
> Ah, it looks like you are following the existing `CloexecCreatCheck` check, 
> no need to do it in this patch. 
"prefer pipe2() with O_CLOEXEC to avoid leaking file descriptors to child 
processes"

No need to change in this patch if you're following an existing pattern, but 
please do update the message in all checks in a separate patch to explain 
things better.



Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h:18
+
+/// pipe() is better to be replaced by pipe2().
+///

"Suggests to replace calls to pipe() with calls to pipe2()."



Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst:6
+
+Detects usage of ``pipe()``. The usage of ``pipe()`` is not recommended, it's 
better to use ``pipe2()``.
+Without this flag, an opened sensitive file descriptor would remain open across

WDYT about this rewrite?

"""
This check detects usage of pipe().  Using pipe() is not recommended, pipe2() 
is the suggested replacement.

The check also adds the O_CLOEXEC flag that marks the file descriptor to be 
closed in child processes.  Without this flag a sensitive file descriptor can 
be leaked to a child process, potentially into a lower-privileged SELinux 
domain.
"""



Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst:16
+
+  // becomes
+

"Suggested replacement"

Also use a separate code block for the replacement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

Oh, this ticket is not going to die from neglect. It is true that D43515 
 is a higher priority, but I need this 
IRBuilder work done as well. My department head wanted it done by the end of 
_last_ year. It's not going to die.

How about I merge your changes into this ticket and we continue work over here? 
There is the issue of the documentation that lebedev.ri asked you to write. Can 
I talk you into putting that together and sending it to me 
? I'll work on the documentation that I was asked to write. 
Between the two of us we should be in pretty good shape. Does that work for you?

I am still waiting for feedback from an actual consumer of the IRBuilder who 
will be using this new functionality. If someone clang-side could chime in on 
this ticket I'd very much appreciate it.


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

https://reviews.llvm.org/D53157



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

Very nice testing approach!




Comment at: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h:20
+/// Provides notifications for file changes in a directory.
+
+/// Invokes client-provided function on every filesystem event in the watched

Looks like triple slashes on empty lines got removed, splitting the doc comment.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40
+// FDRead.
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {

Three slashes for doc comments?



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:40
+// FDRead.
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {

gribozavr wrote:
> Three slashes for doc comments?
Don't write what something is used for currently, such comments go out of date 
quickly.  (Just delete the last sentence.)



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:41
+// Currently used just for one-off termination signal.
+struct SemaphorPipe {
+  // Expecting two file-descriptors opened as a pipe in the canonical POSIX

Semaphor*e*



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:42
+struct SemaphorPipe {
+  // Expecting two file-descriptors opened as a pipe in the canonical POSIX
+  // order: pipefd[0] refers to the read end of the pipe. pipefd[1] refers to

"Expects"

Three slashes for doc comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:49
+assert(Result != -1);
+  };
+  ~SemaphorPipe() {

Extra semicolon.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:52
+close(FDWrite);
+close(FDRead);
+  }

Since it closes the file descriptors in the destructor, I feel like it should 
also be responsible for calling `pipe` in the constructor.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:58
+
+// Mutex-protected queue of Events.
+class EventQueue {

Three slashes for doc comments.



Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:120
+
+  // Consuming inotify events and pushing events to the Queue.
+  void InotifyPollingLoop();

"consumes", "pushes"



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:109
+
+  // This method is used by DirectoryWatcher
+  void consume(DirectoryWatcher::Event E, bool IsInitial) {

Please add a period at the end of the comment (everywhere in the patch).



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:153
+  // Not locking - caller has to lock Mtx
+  llvm::Optional Result() const {
+if (ExpectedInitial.empty() && ExpectedNonInitial.empty() &&

Please name functions consistently -- there's both `consume()` that starts with 
lowercase, and `Result()` that starts with uppercase.

Please refer to the current naming rules in the style guide and apply 
everywhere in the patch.





Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:210
+
+  // TestConsumer didn't reach the expected state in given time.
+  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==

"If the following assertions fail, it is a sign that ..."

Also you can stream the message into the EXPECT_TRUE, it will be printed if the 
assertion fails.

EXPECT_TRUE(...) << "whatever";



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:224
+
+TEST(DirectoryWatcherTest, initialScanSync) {
+  DirectoryWatcherTestFixture fixture;

Test names start with an uppercase letter (`InitialScanSync`).  Please apply 
everywhere in the patch.



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:243
+  },
+  true);
+

Add /*waitForInitialSync=*/ ?  (everywhere in the patch)



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:304
+  {
+  {DirectoryWatcher::Event::EventKind::Modified, "a"},
+  }};

Delete the comma and wrap onto one line.



Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:419
+
+std::error_code setTimeRes = 
llvm::sys::fs::setLastAccessAndModificationTime(FD, NewTimePt,
+NewTimePt);

80 columns.


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

https://reviews.llvm.org/D58418



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

r362243 - Suppress nothrow/Exception spec conflict warning when we dont know the ES.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May 31 09:46:38 2019
New Revision: 362243

URL: http://llvm.org/viewvc/llvm-project?rev=362243=rev
Log:
Suppress nothrow/Exception spec conflict warning when we dont know the ES.

In any situation where the Exception Spec isn't clear, suppress the
warning to avoid false positives.

Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=362243=362242=362243=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri May 31 09:46:38 2019
@@ -6976,18 +6976,18 @@ static bool handleFunctionTypeAttr(TypeP
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:
   case EST_NoThrow:
-  case EST_Unparsed:
 // Exception spec doesn't conflict with nothrow, so don't warn.
-// Unparsed is included in this, since method signatures aren't parsed
-// until after the fact.
+LLVM_FALLTHROUGH;
+  case EST_Unparsed:
+  case EST_Uninstantiated:
+  case EST_DependentNoexcept:
+  case EST_Unevaluated:
+// We don't have enough information to properly determine if there is a
+// conflict, so suppress the warning.
 break;
-
   case EST_Dynamic:
   case EST_MSAny:
   case EST_NoexceptFalse:
-  case EST_DependentNoexcept:
-  case EST_Unevaluated:
-  case EST_Uninstantiated:
 S.Diag(attr.getLoc(), diag::warn_nothrow_attribute_ignored);
 break;
   }

Modified: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp?rev=362243=362242=362243=diff
==
--- cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp (original)
+++ cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp Fri May 31 09:46:38 
2019
@@ -54,6 +54,9 @@ __declspec(nothrow) void foo5() noexcept
 // expected-warning@+1{{'nothrow' attribute conflicts with exception 
specification; attribute ignored}}
 __declspec(nothrow) void foo6() noexcept(noexcept(foo3()));
 
+template
+__declspec(nothrow) void foo7() noexcept(noexcept(F()));
+
 // FIXME: It would be nice to be able to warn on these, however at the time we
 // evaluate the nothrow, these have yet to be parsed, so the data is not yet
 // there.


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

[PATCH] D61697: [lit] Disable test on darwin when building shared libs.

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61697



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

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

[Nikolaus Wittenstein via Phabricator via cfe-commits]

adzenith added a comment.

It appears I was mistaken - the `(?-i)` caused the regex match to fail 
entirely, which made the system header move all the way to the end. It didn't 
actually solve my problem.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33932



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

[PATCH] D62739: AMDGPU: Always emit amdgpu-flat-work-group-size

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: t-tye, b-sumner.
Herald added subscribers: tpr, dstuttard, yaxunl, nhaehnle, wdng, jvesely, 
kzhuravl.

The backend default maximum should be the hardware maximum, so the
frontend should set the implementation defined default maximum.


https://reviews.llvm.org/D62739

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenOpenCL/amdgpu-attrs.cl


Index: test/CodeGenOpenCL/amdgpu-attrs.cl
===
--- test/CodeGenOpenCL/amdgpu-attrs.cl
+++ test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -143,6 +143,10 @@
 // CHECK: define void @a_function() [[A_FUNCTION:#[0-9]+]]
 }
 
+kernel void default_kernel() {
+// CHECK: define amdgpu_kernel void @default_kernel() 
[[DEFAULT_KERNEL_ATTRS:#[0-9]+]]
+}
+
 
 // Make sure this is silently accepted on other targets.
 // X86-NOT: "amdgpu-flat-work-group-size"
@@ -161,20 +165,20 @@
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_64_64]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="64,64" 
"amdgpu-implicitarg-num-bytes"="48"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_16_128]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="16,128" 
"amdgpu-implicitarg-num-bytes"="48"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { convergent noinline nounwind 
optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { convergent noinline nounwind 
optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[NUM_SGPR_32]] = { convergent noinline nounwind 
optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32"
-// CHECK-DAG: attributes [[NUM_VGPR_64]] = { convergent noinline nounwind 
optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-vgpr"="64"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { convergent noinline nounwind 
optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { convergent noinline nounwind 
optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2,4"
+// CHECK-DAG: attributes [[NUM_SGPR_32]] = { convergent noinline nounwind 
optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32"
+// CHECK-DAG: attributes [[NUM_VGPR_64]] = { convergent noinline nounwind 
optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-vgpr"="64"
 
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2]] = { 
convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4]] = { 
convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-waves-per-eu"="2,4"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_SGPR_32]] = { 
convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_VGPR_64]] = { 
convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-vgpr"="64"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_SGPR_32]] = { convergent 
noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48" 
"amdgpu-num-sgpr"="32" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_VGPR_64]] = { convergent 
noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48" 
"amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4_NUM_SGPR_32]] = { convergent 
noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48" 
"amdgpu-num-sgpr"="32" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4_NUM_VGPR_64]] = { convergent 
noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48" 
"amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[NUM_SGPR_32_NUM_VGPR_64]] = { convergent noinline 
nounwind optnone "amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32" 
"amdgpu-num-vgpr"="64"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_SGPR_32]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-sgpr"="32" 
"amdgpu-waves-per-eu"="2"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_VGPR_64]] = { convergent 
noinline nounwind optnone "amdgpu-flat-work-group-size"="128,256" 
"amdgpu-implicitarg-num-bytes"="48" "amdgpu-num-vgpr"="64" 

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:311
+  }
+}
+if (CEArg->isStdInitListInitialization())

So `Foo(Bar{1, 2})` is not problematic, it is just that we can't tell it apart 
from `Foo({1,2})`?



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:457
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));

hokein wrote:
> gribozavr wrote:
> > Why do we print the warning if the fixit is not fixing anything?
> We may still want to inform users, these cases can still be converted to 
> `make_unique`, but the check currently is not smart enough to give correct 
> fixes.
Sorry I don't understand -- so what the file used to say, 
`std::make_unique(E{1, 2}, 1)`, is correct, and this patch introduces a 
regression?  Or is some other syntax necessary?

Please add FIXMEs anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

[Nikolaus Wittenstein via Phabricator via cfe-commits]

adzenith added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

I just came across this case-insensitive behavior when trying to regroup 
include blocks and it was surprising. I'm trying to put system headers after 
other angle-bracket includes (like for Qt), and I was trying:

  - Regex:   '^<[a-z_]*>'
Priority:5
  - Regex:   '^<[A-Za-z_]*>'
Priority:4

Unfortunately, `#include ` matches the first regex and so Qt headers 
get sorted in with system headers. Would it be possible to add the option to do 
case-sensitive matching after all? Should I open a new issue?

For anyone else who wants case-sensitive matching, I just did discover that 
`(?-i)` seems to do the trick. I thought it wouldn't because the docs mention 
extended regex, but I guess it's more extended than I thought! Using 
`'(?-i)^<[a-z_]*>'` does the trick for me.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33932



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

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 3 inline comments as done.
mibintc added inline comments.



Comment at: include/llvm/IR/FPState.h:9-31
+  enum FPModelKind {
+FPM_Off,
+FPM_Precise,
+FPM_Strict,
+FPM_Fast
+  };
+

kpn wrote:
> lebedev.ri wrote:
> > All this needs comments, and possibly better names.
> > `FPM_Off`,etc is very non-self-explanatory.
> Shouldn't there be an "accept default" value? That way 
> CreateConstrainedFAdd()/CreateConstrainedBinOp() can accept enumerations and 
> avoid having front ends having to deal with MDNode*.
By "off" I just meant, not specified, and therefore the default setting would 
prevail.  If you prefer "default" instead of "off" that's fine by me. 



Comment at: include/llvm/IR/IRBuilder.h:247
+switch (DefaultConstrainedExcept) {
+case FPState::CE_Off:
+case FPState::CE_Strict:

In order to match the code that @kpn wrote, I should set 
DefaultConstrainedExcept = strict instead of just leaving it off.



Comment at: include/llvm/IR/IRBuilder.h:261
+switch (DefaultConstrainedRounding) {
+case FPState::CR_Off:
+case FPState::CR_Dynamic:

to match @kpn patch, I should set DefaultConstrainedRounding to dynamic instead 
of leaving it off


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730



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

r362236 - Suppress nothrow/exception spec conflict warning when ES is parsed.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May 31 08:56:27 2019
New Revision: 362236

URL: http://llvm.org/viewvc/llvm-project?rev=362236=rev
Log:
Suppress nothrow/exception spec conflict warning when ES is parsed.

The previously added warning ended up causing false positives when
nothrow was used on member functions, where the exception specification
wasn't yet parsed.  So, throw() and noexcept(true) both were incorrectly
warning.  There doesn't seem to be a good way to force these to be parsed
to identify which they are (and likely should not be), so suppress the warning.

For now, unevaluated/uninstantiated are left as warnings as I am not
creative enough to find a reproducer that causes a false positive for
either.

Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=362236=362235=362236=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri May 31 08:56:27 2019
@@ -6976,7 +6976,10 @@ static bool handleFunctionTypeAttr(TypeP
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:
   case EST_NoThrow:
+  case EST_Unparsed:
 // Exception spec doesn't conflict with nothrow, so don't warn.
+// Unparsed is included in this, since method signatures aren't parsed
+// until after the fact.
 break;
 
   case EST_Dynamic:
@@ -6985,7 +6988,6 @@ static bool handleFunctionTypeAttr(TypeP
   case EST_DependentNoexcept:
   case EST_Unevaluated:
   case EST_Uninstantiated:
-  case EST_Unparsed:
 S.Diag(attr.getLoc(), diag::warn_nothrow_attribute_ignored);
 break;
   }

Modified: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp?rev=362236=362235=362236=diff
==
--- cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp (original)
+++ cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp Fri May 31 08:56:27 
2019
@@ -53,3 +53,16 @@ __declspec(nothrow) void foo4() noexcept
 __declspec(nothrow) void foo5() noexcept(noexcept(foo2()));
 // expected-warning@+1{{'nothrow' attribute conflicts with exception 
specification; attribute ignored}}
 __declspec(nothrow) void foo6() noexcept(noexcept(foo3()));
+
+// FIXME: It would be nice to be able to warn on these, however at the time we
+// evaluate the nothrow, these have yet to be parsed, so the data is not yet
+// there.
+struct S {
+  __declspec(nothrow) void f1();
+#ifndef CPP17
+  __declspec(nothrow) void f2() throw();
+  __declspec(nothrow) void f3() throw(int);
+#endif
+  __declspec(nothrow) void f4() noexcept(true);
+  __declspec(nothrow) void f5() noexcept(false);
+};


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

[PATCH] D62738: [HIP] Support texture type

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a reviewer: a.sidorin.

This patch handles `__attribute__((device_builtin_vector_type))`  for HIP.

If a class or struct type has this attribute, any variables with this type will
be emitted as global symbol in device code with undef initializer, and
registered by `__hipRegisterVar` with shadow variable in host code.

This allows HIP runtime to implement support of HIP texture type.

For CUDA, there is no change for codegen since it depends on implementation.


https://reviews.llvm.org/D62738

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCUDA/texture.cu
  test/SemaCUDA/attr-declspec.cu
  test/SemaCUDA/attributes-on-non-cuda.cu

Index: test/SemaCUDA/attributes-on-non-cuda.cu
===
--- test/SemaCUDA/attributes-on-non-cuda.cu
+++ test/SemaCUDA/attributes-on-non-cuda.cu
@@ -7,11 +7,12 @@
 // RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyntax-only -verify -x c %s
 
 #if defined(EXPECT_WARNINGS)
-// expected-warning@+12 {{'device' attribute ignored}}
-// expected-warning@+12 {{'global' attribute ignored}}
-// expected-warning@+12 {{'constant' attribute ignored}}
-// expected-warning@+12 {{'shared' attribute ignored}}
-// expected-warning@+12 {{'host' attribute ignored}}
+// expected-warning@+13 {{'device' attribute ignored}}
+// expected-warning@+13 {{'global' attribute ignored}}
+// expected-warning@+13 {{'constant' attribute ignored}}
+// expected-warning@+13 {{'shared' attribute ignored}}
+// expected-warning@+13 {{'host' attribute ignored}}
+// expected-warning@+20 {{'device_builtin_texture_type' attribute ignored}}
 //
 // NOTE: IgnoredAttr in clang which is used for the rest of
 // attributes ignores LangOpts, so there are no warnings.
Index: test/SemaCUDA/attr-declspec.cu
===
--- test/SemaCUDA/attr-declspec.cu
+++ test/SemaCUDA/attr-declspec.cu
@@ -6,11 +6,12 @@
 // RUN: %clang_cc1 -DEXPECT_WARNINGS -fms-extensions -fsyntax-only -verify -x c %s
 
 #if defined(EXPECT_WARNINGS)
-// expected-warning@+12 {{'__device__' attribute ignored}}
-// expected-warning@+12 {{'__global__' attribute ignored}}
-// expected-warning@+12 {{'__constant__' attribute ignored}}
-// expected-warning@+12 {{'__shared__' attribute ignored}}
-// expected-warning@+12 {{'__host__' attribute ignored}}
+// expected-warning@+13 {{'__device__' attribute ignored}}
+// expected-warning@+13 {{'__global__' attribute ignored}}
+// expected-warning@+13 {{'__constant__' attribute ignored}}
+// expected-warning@+13 {{'__shared__' attribute ignored}}
+// expected-warning@+13 {{'__host__' attribute ignored}}
+// expected-warning@+19 {{'__device_builtin_texture_type__' attribute ignored}}
 //
 // (Currently we don't for the other attributes. They are implemented with
 // IgnoredAttr, which is ignored irrespective of any LangOpts.)
Index: test/CodeGenCUDA/texture.cu
===
--- /dev/null
+++ test/CodeGenCUDA/texture.cu
@@ -0,0 +1,28 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -std=c++11 -fcuda-is-device \
+// RUN: -emit-llvm -o - %s | FileCheck -check-prefixes=CUDADEV %s
+// RUN: %clang_cc1 -triple x86_64 -std=c++11 \
+// RUN: -emit-llvm -o - %s | FileCheck -check-prefixes=CUDAHOST %s
+
+// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -std=c++11 -fvisibility hidden -fapply-global-visibility-to-externs \
+// RUN: -emit-llvm -o - -x hip %s | FileCheck -check-prefixes=HIPDEV %s
+// RUN: %clang_cc1 -triple x86_64 -std=c++11 \
+// RUN: -emit-llvm -o - -x hip %s | FileCheck -check-prefixes=HIPHOST %s
+
+struct textureReference {
+  int a;
+};
+
+template 
+struct __attribute__((device_builtin_texture_type)) texture : public textureReference {
+texture() { a = 1; }
+};
+
+texture tex;
+// CUDADEV-NOT: @tex
+// CUDAHOST-NOT: call i32 @__hipRegisterVar{{.*}}@tex
+// HIPDEV: @tex = protected addrspace(1) global %struct.texture undef
+// HIPDEV-NOT: declare{{.*}}void @_ZN7textureIfLi2ELi1EEC1Ev
+// HIPHOST:  define{{.*}}@_ZN7textureIfLi2ELi1EEC1Ev
+// HIPHOST:  call i32 @__hipRegisterVar{{.*}}@tex
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6786,6 +6786,9 @@
   case ParsedAttr::AT_CUDAHost:
 handleSimpleAttributeWithExclusions(S, D, AL);
 break;
+  case ParsedAttr::AT_CUDADeviceBuiltinTextureType:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_GNUInline:
 handleGNUInlineAttr(S, D, AL);
 break;
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ 

[PATCH] D62582: [CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Oops, we weren't handling the case when the newly-added overload is dominated 
by an existing one. Fixed.




Comment at: lib/Sema/SemaCodeComplete.cpp:1300
+auto  =
+OverloadMap[std::make_pair(CurContext, Method->getName())];
+for (const DeclIndexPair& Entry : OverloadSet) {

ilya-biryukov wrote:
> Won't this crash on `Method->getName()`? Could we add a test for that?
> Completing in a class that has a user-defined `operator==()` would probably 
> do the trick.
Right! Fixed and test added. We now no longer hash the strings which is nice 
too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62582



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

[PATCH] D62582: [CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 202441.
sammccall marked 2 inline comments as done.
sammccall added a comment.

Handle methods without "normal" identifier names.
Add comments.
Trigger in both directions (dominated and dominated-by).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62582

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/member-access.cpp

Index: test/CodeCompletion/member-access.cpp
===
--- test/CodeCompletion/member-access.cpp
+++ test/CodeCompletion/member-access.cpp
@@ -210,3 +210,66 @@
 // CHECK-CC9: memfun2 (InBase) : [#void#][#Base3::#]memfun2(<#int#>) (requires fix-it: {181:4-181:5} to "->")
 // CHECK-CC9: memfun3 : [#int#]memfun3(<#int#>) (requires fix-it: {181:4-181:5} to "->")
 // CHECK-CC9: operator-> : [#Derived *#]operator->()[# const#]
+
+// These overload sets differ only by return type and this-qualifiers.
+// So for any given callsite, only one is available.
+struct Overloads {
+  double ConstOverload(char);
+  int ConstOverload(char) const;
+
+  int RefOverload(char) &;
+  double RefOverload(char) const&;
+  char RefOverload(char) &&;
+};
+void testLValue(Overloads& Ref) {
+  Ref.
+}
+void testConstLValue(const Overloads& ConstRef) {
+  ConstRef.
+}
+void testRValue() {
+  Overloads().
+}
+void testXValue(Overloads& X) {
+  static_cast(X).
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:225:7 %s -o - | FileCheck -check-prefix=CHECK-LVALUE %s \
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \
+// RUN: --implicit-check-not="[#double#]RefOverload(" \
+// RUN: --implicit-check-not="[#char#]RefOverload("
+// CHECK-LVALUE-DAG: [#double#]ConstOverload(
+// CHECK-LVALUE-DAG: [#int#]RefOverload(
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:228:12 %s -o - | FileCheck -check-prefix=CHECK-CONSTLVALUE %s \
+// RUN: --implicit-check-not="[#double#]ConstOverload(" \
+// RUN: --implicit-check-not="[#int#]RefOverload(" \
+// RUN: --implicit-check-not="[#char#]RefOverload("
+// CHECK-CONSTLVALUE: [#int#]ConstOverload(
+// CHECK-CONSTLVALUE: [#double#]RefOverload(
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:231:15 %s -o - | FileCheck -check-prefix=CHECK-PRVALUE %s \
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \
+// RUN: --implicit-check-not="[#int#]RefOverload(" \
+// RUN: --implicit-check-not="[#double#]RefOverload("
+// CHECK-PRVALUE: [#double#]ConstOverload(
+// CHECK-PRVALUE: [#char#]RefOverload(
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:234:31 %s -o - | FileCheck -check-prefix=CHECK-XVALUE %s \
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \
+// RUN: --implicit-check-not="[#int#]RefOverload(" \
+// RUN: --implicit-check-not="[#double#]RefOverload("
+// CHECK-XVALUE: [#double#]ConstOverload(
+// CHECK-XVALUE: [#char#]RefOverload(
+
+void testOverloadOperator() {
+  struct S {
+char operator=(int) const;
+int operator=(int);
+  } s;
+  return s.
+}
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:270:12 %s -o - | FileCheck -check-prefix=CHECK-OPER %s \
+// RUN: --implicit-check-not="[#char#]operator=("
+// CHECK-OPER: [#int#]operator=(
+
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -16,7 +16,9 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/QualTypeNames.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
@@ -152,9 +154,16 @@
   /// different levels of, e.g., the inheritance hierarchy.
   std::list ShadowMaps;
 
+  /// Overloaded C++ member functions found by SemaLookup.
+  /// Used to determine when one overload is dominated by another.
+  llvm::DenseMap, ShadowMapEntry>
+  OverloadMap;
+
   /// If we're potentially referring to a C++ member function, the set
   /// of qualifiers applied to the object type.
   Qualifiers ObjectTypeQualifiers;
+  /// The kind of the object expression, for rvalue/lvalue overloads.
+  ExprValueKind ObjectKind;
 
   /// Whether the \p ObjectTypeQualifiers field is active.
   bool HasObjectTypeQualifiers;
@@ -230,8 +239,9 @@
   /// out member functions that aren't available (because there will be a
   /// cv-qualifier mismatch) or prefer functions with an exact qualifier
   /// match.
-  void setObjectTypeQualifiers(Qualifiers Quals) {
+  void setObjectTypeQualifiers(Qualifiers Quals, ExprValueKind Kind) {
 ObjectTypeQualifiers = Quals;
+ObjectKind = Kind;
 HasObjectTypeQualifiers = true;
   }
 
@@ -1157,6 +1167,53 @@
   R.InBaseClass = true;
 }
 
+enum class OverloadCompare { BothViable, Dominates, Dominated };
+// Will Candidate ever be called on the object, when 

r362232 - Revise test case due to the change from CUDA 10+.

[Michael Liao via cfe-commits]

Author: hliao
Date: Fri May 31 08:29:55 2019
New Revision: 362232

URL: http://llvm.org/viewvc/llvm-project?rev=362232=rev
Log:
Revise test case due to the change from CUDA 10+.

Modified:
cfe/trunk/test/Driver/offloading-interoperability.c

Modified: cfe/trunk/test/Driver/offloading-interoperability.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/offloading-interoperability.c?rev=362232=362231=362232=diff
==
--- cfe/trunk/test/Driver/offloading-interoperability.c (original)
+++ cfe/trunk/test/Driver/offloading-interoperability.c Fri May 31 08:29:55 2019
@@ -11,7 +11,7 @@
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE:  clang{{.*}}" "-cc1" "-triple" 
"nvptx64-nvidia-cuda"
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NOT:  -fopenmp
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: ptxas" "-m64"
-// NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: fatbinary" "--cuda" "-64"
+// NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: fatbinary"{{( "--cuda")?}} "-64"
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: clang{{.*}}" "-cc1" "-triple" 
"powerpc64le-unknown-linux-gnu"
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE:  -fopenmp
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE-NEXT: {{ld(.exe)?"}} {{.*}}"-m" "elf64lppc"


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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: include/llvm/IR/IRBuilder.h:234
+  /// Set the exception handling to be used with constrained floating point
+  void setDefaultConstrainedExcept(MDNode *NewExcept) { 
+DefaultConstrainedExcept = NewExcept; 

kpn wrote:
> mibintc wrote:
> > andrew.w.kaylor wrote:
> > > I think it would be better to add some enumerated type to describe the 
> > > exception semantic and rounding modes. The MDNode is an implementation 
> > > detail and an awkward one for the front end to have to deal with.
> > I posted a patch showing the rounding and exception information being 
> > passed as enumeration not MDNode. I've uploaded the patch here, 
> > https://reviews.llvm.org/D62730
> It's a good idea. D62730 beat me to the punch.
BTW I created FPState.h because if I put the enumeration types in IRBuilder it 
dragged in too many include files in clang and required other random source 
changes for disambigutation.  Anyway I don't need to comandeer your patch -- I 
don't want it to die from neglect.  I can close mine? 


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

https://reviews.llvm.org/D53157



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

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added inline comments.



Comment at: include/llvm/IR/FPState.h:9-31
+  enum FPModelKind {
+FPM_Off,
+FPM_Precise,
+FPM_Strict,
+FPM_Fast
+  };
+

lebedev.ri wrote:
> All this needs comments, and possibly better names.
> `FPM_Off`,etc is very non-self-explanatory.
Shouldn't there be an "accept default" value? That way 
CreateConstrainedFAdd()/CreateConstrainedBinOp() can accept enumerations and 
avoid having front ends having to deal with MDNode*.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

I wanted to get the API straight before working on documentation.




Comment at: include/llvm/IR/IRBuilder.h:228
+  /// Enable/Disable use of constrained floating point math
+  void setIsConstrainedFP(bool IsCon) { IsFPConstrained = IsCon; }
+

erichkeane wrote:
> kpn wrote:
> > kbarton wrote:
> > > This is a minor quibble, but the method is setIsConstrainedFP, while the 
> > > member is IsFPConstrained. 
> > > I'm not sure if that is intentionally different, or an oversight. 
> > Yeah, that's an oversight. Fixed.
> IS this fixed?
In my working copy, yes.



Comment at: include/llvm/IR/IRBuilder.h:234
+  /// Set the exception handling to be used with constrained floating point
+  void setDefaultConstrainedExcept(MDNode *NewExcept) { 
+DefaultConstrainedExcept = NewExcept; 

mibintc wrote:
> andrew.w.kaylor wrote:
> > I think it would be better to add some enumerated type to describe the 
> > exception semantic and rounding modes. The MDNode is an implementation 
> > detail and an awkward one for the front end to have to deal with.
> I posted a patch showing the rounding and exception information being passed 
> as enumeration not MDNode. I've uploaded the patch here, 
> https://reviews.llvm.org/D62730
It's a good idea. D62730 beat me to the punch.



Comment at: include/llvm/IR/IRBuilder.h:1249
+if (IsFPConstrained)
+  return CreateConstrainedFAdd(L, R, FMFSource, Name, nullptr, nullptr);
+

erichkeane wrote:
> Why set the last 2 to nullptr when you have defaults for these?
Eh, no really good reason.



Comment at: include/llvm/IR/IRBuilder.h:1259
+  Instruction *FMFSource = nullptr,
   const Twine  = "",
+  MDNode *RoundingMD = nullptr,   
+  MDNode *ExceptMD = nullptr) {

erichkeane wrote:
> The last 2 parameters are never actually used except in the test.  Are these 
> really important to have if they are never used in source?
They're part of the intrinsics. It seems like there should be a way to emit 
them as part of the instruction when creating the instruction.



Comment at: include/llvm/IR/IRBuilder.h:1408
 
+  CallInst *CreateConstrainedFRem(Value *L, Value *R, 
+  Instruction *FMFSource = nullptr, 

erichkeane wrote:
> All these CreateConstrainedXXX should be distilled down to a single function 
> that takes the intrinsic as a parameter.
That moves us further down the road towards having modes. I haven't seen a 
front-end guy sign off on it yet, but it seems like a good idea.


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

https://reviews.llvm.org/D53157



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

Re: r278642 - P0217R3: code generation support for decomposition declarations.

[Nico Weber via cfe-commits]

The invented mangling for clang-cl turns out to not be correct :)
https://bugs.llvm.org/show_bug.cgi?id=42093

Maybe it's better to add a "can't yet mangle" error, so that we don't
silently do the wrong thing?

On Sun, Aug 14, 2016 at 9:41 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Sun Aug 14 20:33:41 2016
> New Revision: 278642
>
> URL: http://llvm.org/viewvc/llvm-project?rev=278642=rev
> Log:
> P0217R3: code generation support for decomposition declarations.
>
> Added:
> cfe/trunk/test/CodeGenCXX/cxx1z-decomposition.cpp
> Modified:
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/AST/ItaniumMangle.cpp
> cfe/trunk/lib/AST/MicrosoftMangle.cpp
> cfe/trunk/lib/CodeGen/CGDecl.cpp
> cfe/trunk/lib/CodeGen/CGExpr.cpp
> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/SemaCXX/cxx1z-decomposition.cpp
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=278642=278641=278642=diff
>
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Sun Aug 14 20:33:41 2016
> @@ -8721,6 +8721,14 @@ bool ASTContext::DeclMustBeEmitted(const
>!VD->evaluateValue())
>  return true;
>
> +  // Likewise, variables with tuple-like bindings are required if their
> +  // bindings have side-effects.
> +  if (auto *DD = dyn_cast(VD))
> +for (auto *BD : DD->bindings())
> +  if (auto *BindingVD = BD->getHoldingVar())
> +if (DeclMustBeEmitted(BindingVD))
> +  return true;
> +
>return false;
>  }
>
>
> Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=278642=278641=278642=diff
>
> ==
> --- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
> +++ cfe/trunk/lib/AST/ItaniumMangle.cpp Sun Aug 14 20:33:41 2016
> @@ -1195,18 +1195,21 @@ void CXXNameMangler::mangleUnqualifiedNa
>case DeclarationName::Identifier: {
>  const IdentifierInfo *II = Name.getAsIdentifierInfo();
>
> -// We mangle decomposition declarations as the name of their first
> binding.
> +// We mangle decomposition declarations as the names of their
> bindings.
>  if (auto *DD = dyn_cast(ND)) {
> -  auto B = DD->bindings();
> -  if (B.begin() == B.end()) {
> -// FIXME: This is ill-formed but we accept it as an extension.
> -DiagnosticsEngine  = Context.getDiags();
> -unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
> -"cannot mangle global empty decomposition decl");
> -Diags.Report(DD->getLocation(), DiagID);
> -break;
> -  }
> -  II = (*B.begin())->getIdentifier();
> +  // FIXME: Non-standard mangling for decomposition declarations:
> +  //
> +  //   ::= DC * E
> +  //
> +  // These can never be referenced across translation units, so we do
> +  // not need a cross-vendor mangling for anything other than
> demanglers.
> +  // Proposed on cxx-abi-dev on 2016-08-12
> +  Out << "DC";
> +  for (auto *BD : DD->bindings())
> +mangleSourceName(BD->getDeclName().getAsIdentifierInfo());
> +  Out << 'E';
> +  writeAbiTags(ND, AdditionalAbiTags);
> +  break;
>  }
>
>  if (II) {
>
> Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=278642=278641=278642=diff
>
> ==
> --- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
> +++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Sun Aug 14 20:33:41 2016
> @@ -394,7 +394,8 @@ bool MicrosoftMangleContextImpl::shouldM
>if (!getASTContext().getLangOpts().CPlusPlus)
>  return false;
>
> -  if (const VarDecl *VD = dyn_cast(D)) {
> +  const VarDecl *VD = dyn_cast(D);
> +  if (VD && !isa(D)) {
>  // C variables are not mangled.
>  if (VD->isExternC())
>return false;
> @@ -780,6 +781,21 @@ void MicrosoftCXXNameMangler::mangleUnqu
>  }
>}
>
> +  if (const DecompositionDecl *DD = dyn_cast(ND)) {
> +// FIXME: Invented mangling for decomposition declarations:
> +//   [X,Y,Z]
> +// where X,Y,Z are the names of the bindings.
> +llvm::SmallString<128> Name("[");
> +for (auto *BD : DD->bindings()) {
> +  if (Name.size() > 1)
> +Name += ',';
> +  Name += BD->getDeclName().getAsIdentifierInfo()->getName();
> +}
> +Name += ']';
> +mangleSourceName(Name);
> +break;
> +  }
> +
>if (const VarDecl *VD = dyn_cast(ND)) {
>  // We must have an anonymous union or struct declaration.
>  

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Added lit tests for the simple (`Decl`) cases. The `Expr` and type related 
changes are more tricky to do. But I do not like this approach to add new tests 
because the test function in D60463  does 
almost the same thing for every import in ASTTests without adding lot of new 
checks manually.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60499



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

I don't know which patch will proceed; but they weren't added already, this 
seems to be missing documentation changes.


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

https://reviews.llvm.org/D53157



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

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 202438.
balazske added a comment.

- Import BraceRange of EnumDecl.
- Added lit tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60499

Files:
  lib/AST/ASTImporter.cpp
  test/Import/enum/test.cpp
  test/Import/namespace/Inputs/NS.cpp
  test/Import/namespace/test.cpp
  test/Import/struct-and-var/test.cpp
  test/Import/template-specialization/test.cpp

Index: test/Import/template-specialization/test.cpp
===
--- test/Import/template-specialization/test.cpp
+++ test/Import/template-specialization/test.cpp
@@ -1,4 +1,7 @@
-// RUN: clang-import-test -import %S/Inputs/T.cpp -expression %s
+// RUN: clang-import-test -dump-ast -import %S/Inputs/T.cpp -expression %s | FileCheck %s
+
+// CHECK: |-ClassTemplateSpecializationDecl
+// CHECK-SAME:  line:4:20 struct A
 
 void expr() {
   A::B b1;
Index: test/Import/struct-and-var/test.cpp
===
--- test/Import/struct-and-var/test.cpp
+++ test/Import/struct-and-var/test.cpp
@@ -1,4 +1,8 @@
-// RUN: clang-import-test --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s
+// RUN: clang-import-test -dump-ast --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s | FileCheck %s
+
+// CHECK: `-CXXRecordDecl
+// CHECK-SAME: Inputs/S2.cpp:1:1, line:3:1> line:1:8 struct F
+
 void expr() {
   struct F f;
   int x = f.a;
Index: test/Import/namespace/test.cpp
===
--- /dev/null
+++ test/Import/namespace/test.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/NS.cpp -expression %s | FileCheck %s
+
+// CHECK: `-NamespaceDecl
+// CHECK-SAME: Inputs/NS.cpp:1:1, line:5:1> line:1:11 NS
+
+void expr() {
+  static_assert(NS::A == 3);
+}
Index: test/Import/namespace/Inputs/NS.cpp
===
--- /dev/null
+++ test/Import/namespace/Inputs/NS.cpp
@@ -0,0 +1,5 @@
+namespace NS {
+  void f1();
+  void f2();
+  const int A = 3;
+};
Index: test/Import/enum/test.cpp
===
--- test/Import/enum/test.cpp
+++ test/Import/enum/test.cpp
@@ -1,5 +1,7 @@
 // RUN: clang-import-test -dump-ast -import %S/Inputs/S.cpp -expression %s | FileCheck %s
 
+// CHECK: |-EnumDecl
+// CHECK-SAME: Inputs/S.cpp:1:1, line:4:1> line:1:6 E
 // CHECK: OpaqueWithType 'long'
 
 void expr() {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2158,6 +2158,9 @@
   ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
   if (!BeginLocOrErr)
 return BeginLocOrErr.takeError();
+  ExpectedSLoc RBraceLocOrErr = import(D->getRBraceLoc());
+  if (!RBraceLocOrErr)
+return RBraceLocOrErr.takeError();
 
   // Create the "to" namespace, if needed.
   NamespaceDecl *ToNamespace = MergeWithNamespace;
@@ -2167,6 +2170,7 @@
 *BeginLocOrErr, Loc, Name.getAsIdentifierInfo(),
 /*PrevDecl=*/nullptr))
   return ToNamespace;
+ToNamespace->setRBraceLoc(*RBraceLocOrErr);
 ToNamespace->setLexicalDeclContext(LexicalDC);
 LexicalDC->addDeclInternal(ToNamespace);
 
@@ -2464,9 +2468,10 @@
   SourceLocation ToBeginLoc;
   NestedNameSpecifierLoc ToQualifierLoc;
   QualType ToIntegerType;
-  if (auto Imp = importSeq(
-  D->getBeginLoc(), D->getQualifierLoc(), D->getIntegerType()))
-std::tie(ToBeginLoc, ToQualifierLoc, ToIntegerType) = *Imp;
+  SourceRange ToBraceRange;
+  if (auto Imp = importSeq(D->getBeginLoc(), D->getQualifierLoc(),
+   D->getIntegerType(), D->getBraceRange()))
+std::tie(ToBeginLoc, ToQualifierLoc, ToIntegerType, ToBraceRange) = *Imp;
   else
 return Imp.takeError();
 
@@ -2480,6 +2485,7 @@
 
   D2->setQualifierInfo(ToQualifierLoc);
   D2->setIntegerType(ToIntegerType);
+  D2->setBraceRange(ToBraceRange);
   D2->setAccess(D->getAccess());
   D2->setLexicalDeclContext(LexicalDC);
   LexicalDC->addDeclInternal(D2);
@@ -2712,6 +2718,10 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
   if (auto QualifierLocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*QualifierLocOrErr);
   else
@@ -5181,6 +5191,11 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
+
   // Import the qualifier, if any.
   if (auto LocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*LocOrErr);
@@ -6174,7 +6189,8 @@
   TemplateArgumentListInfo *ToResInfo = nullptr;
   if (E->hasExplicitTemplateArgs()) 

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

Incorporating feedback from D53157  is 
probably a good idea. Looks like that hasn't been done yet here completely.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Kevin P. Neal via Phabricator via cfe-commits]

kpn updated this revision to Diff 202436.
kpn marked 4 inline comments as done.
kpn added a comment.

Address review comments.


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

https://reviews.llvm.org/D53157

Files:
  include/llvm/IR/IRBuilder.h
  unittests/IR/IRBuilderTest.cpp

Index: unittests/IR/IRBuilderTest.cpp
===
--- unittests/IR/IRBuilderTest.cpp
+++ unittests/IR/IRBuilderTest.cpp
@@ -122,6 +122,100 @@
   EXPECT_FALSE(II->hasNoNaNs());
 }
 
+TEST_F(IRBuilderTest, ConstrainedFP) {
+  IRBuilder<> Builder(BB);
+  Value *V;
+  CallInst *Call;
+  IntrinsicInst *II;
+
+  V = Builder.CreateLoad(GV);
+
+  Call = Builder.CreateConstrainedFAdd(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+
+  Call = Builder.CreateConstrainedFSub(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fsub);
+
+  Call = Builder.CreateConstrainedFMul(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fmul);
+
+  Call = Builder.CreateConstrainedFDiv(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fdiv);
+
+  Call = Builder.CreateConstrainedFRem(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_frem);
+
+  // Now see if we get constrained intrinsics instead of non-constrained
+  // instructions.
+  Builder.setIsConstrainedFP(true);
+
+  V = Builder.CreateFAdd(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+
+  V = Builder.CreateFSub(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fsub);
+
+  V = Builder.CreateFMul(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fmul);
+  
+  V = Builder.CreateFDiv(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fdiv);
+  
+  V = Builder.CreateFRem(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_frem);
+
+  // Verify the codepaths for setting and overriding the default metadata.
+  MDNode *ExceptStr = MDNode::get(Builder.getContext(), 
+  MDString::get(Builder.getContext(), 
+"fpexcept.strict"));
+  MDNode *ExceptIgn = MDNode::get(Builder.getContext(), 
+  MDString::get(Builder.getContext(), 
+"fpexcept.ignore"));
+  MDNode *RoundDyn = MDNode::get(Builder.getContext(), 
+ MDString::get(Builder.getContext(),
+   "round.dynamic"));
+  MDNode *RoundUp = MDNode::get(Builder.getContext(), 
+MDString::get(Builder.getContext(),
+  "round.upward"));
+
+  V = Builder.CreateFAdd(V, V);
+  ASSERT_TRUE(isa(V));
+  auto *CII = cast(V);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebStrict);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmDynamic);
+
+  Builder.setDefaultConstrainedExcept(ExceptIgn);
+  Builder.setDefaultConstrainedRounding(RoundUp);
+  V = Builder.CreateFAdd(V, V);
+  CII = cast(V);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebIgnore);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmUpward);
+
+  // Now override the defaults.
+  Call = Builder.CreateConstrainedFAdd(V, V, nullptr, "", RoundDyn, ExceptStr);
+  CII = cast(Call);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebStrict);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmDynamic);
+
+  Builder.CreateRetVoid();
+  EXPECT_FALSE(verifyModule(*M));
+}
+
 TEST_F(IRBuilderTest, Lifetime) {
   IRBuilder<> Builder(BB);
   AllocaInst *Var1 = Builder.CreateAlloca(Builder.getInt8Ty());
Index: include/llvm/IR/IRBuilder.h
===
--- include/llvm/IR/IRBuilder.h
+++ include/llvm/IR/IRBuilder.h
@@ -96,13 +96,17 @@
   MDNode *DefaultFPMathTag;
   FastMathFlags FMF;
 
+  bool IsFPConstrained;
+  MDNode *DefaultConstrainedExcept = nullptr;
+  MDNode *DefaultConstrainedRounding = nullptr;
+
   ArrayRef DefaultOperandBundles;
 
 public:
   IRBuilderBase(LLVMContext , MDNode *FPMathTag = nullptr,
 ArrayRef OpBundles = None)
   : Context(context), DefaultFPMathTag(FPMathTag),
-DefaultOperandBundles(OpBundles) {
+IsFPConstrained(false), DefaultConstrainedExcept(nullptr) {
 ClearInsertionPoint();
   }
 
@@ -218,6 +222,40 @@
   /// Set the 

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:304
+// e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around
+// an elidable move constructor.
+if (CEArg->isElidable()) {

gribozavr wrote:
> Isn't it the other way round -- the move constructor is around the init-list 
> constructor?
yeah, this is what I mean, updated the comment.



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:1
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I 
%S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- 
-I %S/Inputs/modernize-smart-ptr
 

gribozavr wrote:
> "-std=c++14-or-later" (will also cover c++2a etc.)
hmm, the test code has some compiler errors on c++2a mode,  it is a different 
issue (I will address it in a follow-up), added a FIXME. 



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:457
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));

gribozavr wrote:
> Why do we print the warning if the fixit is not fixing anything?
We may still want to inform users, these cases can still be converted to 
`make_unique`, but the check currently is not smart enough to give correct 
fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 202434.
hokein marked 6 inline comments as done.
hokein added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I 
%S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- 
-I %S/Inputs/modernize-smart-ptr
+// FIXME: Fix the test code in C++2a mode.
 
 #include "unique_ptr.h"
 #include "initializer_list.h"
@@ -455,10 +455,10 @@
 
   std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES: PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ2.reset(new J(E{1, 2}, 1));
 
   std::unique_ptr PJ3 = std::unique_ptr(new J{ {1, 2}, 1 });
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
@@ -469,10 +469,10 @@
 
   std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ4 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 
1});
   PJ4.reset(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES:  PJ4 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ4.reset(new J{E{1, 2}, 1});
 
   std::unique_ptr FF = std::unique_ptr(new Foo());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning:
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -298,11 +298,20 @@
 return true;
   // Check whether we implicitly construct a class from a
   // std::initializer_list.
-  if (const auto *ImplicitCE = dyn_cast(Arg)) {
-if (ImplicitCE->isStdInitListInitialization())
+  if (const auto *CEArg = dyn_cast(Arg)) {
+// Strip the elidable move constructor, it is present in the AST for
+// C++11/14, e.g. Foo(Bar{1, 2}), the move constructor is around the
+// init-list constructor.
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =
+dyn_cast(TempExp->IgnoreImplicit()))
+  CEArg = UnwrappedCE;
+  }
+}
+if (CEArg->isStdInitListInitialization())
   return true;
   }
-  return false;
 }
 return false;
   };


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
+// FIXME: Fix the test code in C++2a mode.
 
 #include "unique_ptr.h"
 #include "initializer_list.h"
@@ -455,10 +455,10 @@
 
   std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   PJ2.reset(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES: PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ2.reset(new J(E{1, 2}, 1));
 
   std::unique_ptr PJ3 = std::unique_ptr(new J{ {1, 2}, 1 });
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
@@ -469,10 +469,10 @@
 
   std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr 

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 202433.
SjoerdMeijer added a comment.

This time with tests.


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

https://reviews.llvm.org/D60697

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/armv8.1m.main.c
  clang/test/Driver/armv8.1m.main.s
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp

Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -484,22 +484,100 @@
   return StringRef();
 }
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool stripNegationPrefix(StringRef ) {
+  if (Name.startswith("no")) {
+Name = Name.substr(2);
+return true;
   }
+  return false;
+}
+
+StringRef ARM::getArchExtFeature(StringRef ArchExt) {
+  bool Negated = stripNegationPrefix(ArchExt);
   for (const auto AE : ARCHExtNames) {
 if (AE.Feature && ArchExt == AE.getName())
-  return StringRef(AE.Feature);
+  return StringRef(Negated ? AE.NegFeature : AE.Feature);
   }
 
   return StringRef();
 }
 
+static unsigned findDoublePrecisionFPU(unsigned InputFPUKind) {
+  const ARM::FPUName  = ARM::FPUNames[InputFPUKind];
+
+  // If the input FPU already supports double-precision, then there
+  // isn't any different FPU we can return here.
+  //
+  // The current available FPURestriction values are None (no
+  // restriction), D16 (only 16 d-regs) and SP_D16 (16 d-regs
+  // and single precision only); there's no value representing
+  // SP restriction without D16. So this test just means 'is it
+  // SP only?'.
+  if (InputFPU.Restriction != ARM::FPURestriction::SP_D16)
+return ARM::FK_INVALID;
+
+  // Otherwise, look for an FPU entry with all the same fields, except
+  // that SP_D16 has been replaced with just D16, representing adding
+  // double precision and not changing anything else.
+  for (const ARM::FPUName  : ARM::FPUNames) {
+if (CandidateFPU.FPUVer == InputFPU.FPUVer &&
+CandidateFPU.NeonSupport == InputFPU.NeonSupport &&
+CandidateFPU.Restriction == ARM::FPURestriction::D16) {
+  return CandidateFPU.ID;
+}
+  }
+
+  // nothing found
+  return ARM::FK_INVALID;
+}
+
+bool ARM::appendArchExtFeatures(
+  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+  std::vector ) {
+  StringRef StandardFeature = getArchExtFeature(ArchExt);
+  if (!StandardFeature.empty()) {
+Features.push_back(StandardFeature);
+return true;
+  }
+
+  bool Negated = stripNegationPrefix(ArchExt);
+
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+
+if (ArchExt == "fp.dp") {
+  unsigned DoubleFPUKind = findDoublePrecisionFPU(getDefaultFPU(CPU, AK));
+
+  // If the default FPU already supports double-precision, or if
+  // there is no double-prec FPU that extends it, then "fp.dp"
+  // doesn't have a separate meaning, and we treat it as an
+  // invalid extension name.
+  if (DoubleFPUKind == FK_INVALID)
+return false;
+
+  // If there _is_ a separate double-precision FPU, then "nofp.dp"
+  // should disable just the double-precision extension, leaving
+  // the base FPU still enabled if it previously was.
+  if (Negated) {
+Features.push_back("-fp64");
+return true;
+  }
+
+  // Otherwise, select the double-precision FPU.
+  FPUKind = DoubleFPUKind;
+} else if (Negated) {
+  FPUKind = ARM::FK_NONE;
+} else {
+  FPUKind = getDefaultFPU(CPU, AK);
+  if (FPUKind == ARM::FK_NONE)
+return false;
+}
+return ARM::getFPUFeatures(FPUKind, Features);
+  }
+
+  return false;
+}
+
 StringRef ARM::getHWDivName(unsigned HWDivKind) {
   for (const auto D : HWDivNames) {
 if (HWDivKind == D.ID)
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ -240,6 +240,8 @@
 StringRef getSubArch(ArchKind AK);
 StringRef getArchExtName(unsigned ArchExtKind);
 StringRef getArchExtFeature(StringRef ArchExt);
+bool appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+   std::vector );
 StringRef getHWDivName(unsigned HWDivKind);
 
 // Information by Name
Index: clang/test/Driver/armv8.1m.main.s
===
--- clang/test/Driver/armv8.1m.main.s
+++ clang/test/Driver/armv8.1m.main.s
@@ -5,8 +5,14 @@
 # RUN:  FileCheck --check-prefix=ERROR-V81M < %t %s
 # RUN: not %clang -c 

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:302
+  if (const auto *CEArg = dyn_cast(Arg)) {
+// Strip the ediable move constructor, it is presented in C++11/14 
mode.
+// e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around

s/ediable/elidable/

s/presented/present in the AST/



Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:304
+// e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around
+// an elidable move constructor.
+if (CEArg->isElidable()) {

Isn't it the other way round -- the move constructor is around the init-list 
constructor?



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:1
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I 
%S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- 
-I %S/Inputs/modernize-smart-ptr
 

"-std=c++14-or-later" (will also cover c++2a etc.)



Comment at: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp:457
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));

Why do we print the warning if the fixit is not fixing anything?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62736



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

[clang-tools-extra] r362226 - [clangd] Add missing license for rename.cpp, NFC.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri May 31 07:38:16 2019
New Revision: 362226

URL: http://llvm.org/viewvc/llvm-project?rev=362226=rev
Log:
[clangd] Add missing license for rename.cpp, NFC.

Modified:
clang-tools-extra/trunk/clangd/refactor/Rename.cpp

Modified: clang-tools-extra/trunk/clangd/refactor/Rename.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Rename.cpp?rev=362226=362225=362226=diff
==
--- clang-tools-extra/trunk/clangd/refactor/Rename.cpp (original)
+++ clang-tools-extra/trunk/clangd/refactor/Rename.cpp Fri May 31 07:38:16 2019
@@ -1,3 +1,11 @@
+//===--- Rename.cpp - Symbol-rename refactorings -*- 
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 "refactor/Rename.h"
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
@@ -65,7 +73,7 @@ renameWithinFile(ParsedAST , llvm::S
   // Right now we only support renaming the main file, so we
   // drop replacements not for the main file. In the future, we might
   // also support rename with wider scope.
-  // Rename sometimes returns duplicate edits (which is a bug). A side-effect 
of 
+  // Rename sometimes returns duplicate edits (which is a bug). A side-effect 
of
   // adding them to a single Replacements object is these are deduplicated.
   for (const tooling::AtomicChange  : ResultCollector.Result->get()) {
 for (const auto  : Change.getReplacements()) {


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

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Documentation missing. All the blurb from patch description should be in `doc/`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D62736: [clang-tidy] Fix make-unique check to work in C++17 mode.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: gribozavr.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Previously, we intended to omit the check fix to the case when constructor has
any braced-init-list argument. But the HasListInitializedArgument was not
correct to handle all cases (Foo(Bar{1, 2}) will return false in C++14
mode).

This patch fixes it, corrects the tests, and makes the check to run at C++17 
mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62736

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I 
%S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- 
-I %S/Inputs/modernize-smart-ptr
 
 #include "unique_ptr.h"
 #include "initializer_list.h"
@@ -455,10 +454,10 @@
 
   std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 
1));
   PJ2.reset(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES: PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ2.reset(new J(E{1, 2}, 1));
 
   std::unique_ptr PJ3 = std::unique_ptr(new J{ {1, 2}, 1 });
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
@@ -469,10 +468,10 @@
 
   std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ4 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ4 = std::unique_ptr(new J{E{1, 2}, 
1});
   PJ4.reset(new J{E{1, 2}, 1});
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES:  PJ4 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ4.reset(new J{E{1, 2}, 1});
 
   std::unique_ptr FF = std::unique_ptr(new Foo());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning:
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -298,11 +298,20 @@
 return true;
   // Check whether we implicitly construct a class from a
   // std::initializer_list.
-  if (const auto *ImplicitCE = dyn_cast(Arg)) {
-if (ImplicitCE->isStdInitListInitialization())
+  if (const auto *CEArg = dyn_cast(Arg)) {
+// Strip the ediable move constructor, it is presented in C++11/14 
mode.
+// e.g. Foo(Bar{1, 2}), the Bar init-list constructor is wrapped around
+// an elidable move constructor.
+if (CEArg->isElidable()) {
+  if (const auto *TempExp = CEArg->getArg(0)) {
+if (const auto *UnwrappedCE =
+dyn_cast(TempExp->IgnoreImplicit()))
+  CEArg = UnwrappedCE;
+  }
+}
+if (CEArg->isStdInitListInitialization())
   return true;
   }
-  return false;
 }
 return false;
   };


Index: clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++14 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- -I %S/Inputs/modernize-smart-ptr
 
 #include "unique_ptr.h"
 #include "initializer_list.h"
@@ -455,10 +454,10 @@
 
   std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: std::unique_ptr PJ2 = std::unique_ptr(new J(E{1, 2}, 1));
   PJ2.reset(new J(E{1, 2}, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
-  // CHECK-FIXES: PJ2 = std::make_unique(E{1, 2}, 1);
+  // CHECK-FIXES: PJ2.reset(new J(E{1, 2}, 1));
 
   std::unique_ptr PJ3 = std::unique_ptr(new J{ {1, 2}, 1 });
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
@@ -469,10 

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

New files are missing header blurbs; the **precise** explanation of what is 
what is missing (should be in `doc/`, not patch description)




Comment at: include/llvm/IR/FPState.h:9-31
+  enum FPModelKind {
+FPM_Off,
+FPM_Precise,
+FPM_Strict,
+FPM_Fast
+  };
+

All this needs comments, and possibly better names.
`FPM_Off`,etc is very non-self-explanatory.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62730



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

I've posted a small change to this patch here, https://reviews.llvm.org/D62730 
and there's a patch to add clang fp options here, 
https://reviews.llvm.org/D62731




Comment at: include/llvm/IR/IRBuilder.h:234
+  /// Set the exception handling to be used with constrained floating point
+  void setDefaultConstrainedExcept(MDNode *NewExcept) { 
+DefaultConstrainedExcept = NewExcept; 

andrew.w.kaylor wrote:
> I think it would be better to add some enumerated type to describe the 
> exception semantic and rounding modes. The MDNode is an implementation detail 
> and an awkward one for the front end to have to deal with.
I posted a patch showing the rounding and exception information being passed as 
enumeration not MDNode. I've uploaded the patch here, 
https://reviews.llvm.org/D62730


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

https://reviews.llvm.org/D53157



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

r362225 - Fix for PR42089, regression from r362119

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri May 31 07:26:19 2019
New Revision: 362225

URL: http://llvm.org/viewvc/llvm-project?rev=362225=rev
Log:
Fix for PR42089, regression from r362119

The implementation of the NoThrow ExceptionSpecificationType missed a
switch statement for forming the diagnostic when an out-of-line member
redeclaration misses the exception specification.  This patch adds the
correct case statement.

Modified:
cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp

Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=362225=362224=362225=diff
==
--- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Fri May 31 07:26:19 2019
@@ -381,6 +381,11 @@ bool Sema::CheckEquivalentExceptionSpec(
 // when declaring a replaceable global allocation function.
 DiagID = diag::ext_missing_exception_specification;
 ReturnValueOnError = false;
+  } else if (ESI.Type == EST_NoThrow) {
+// Allow missing attribute 'nothrow' in redeclarations, since this is a 
very
+// common omission.
+DiagID = diag::ext_missing_exception_specification;
+ReturnValueOnError = false;
   } else {
 DiagID = diag::err_missing_exception_specification;
 ReturnValueOnError = true;
@@ -421,7 +426,9 @@ bool Sema::CheckEquivalentExceptionSpec(
 OldProto->getNoexceptExpr()->printPretty(OS, nullptr, getPrintingPolicy());
 OS << ")";
 break;
-
+  case EST_NoThrow:
+OS <<"__attribute__((nothrow))";
+break;
   default:
 llvm_unreachable("This spec type is compatible with none.");
   }

Modified: cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp?rev=362225=362224=362225=diff
==
--- cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp (original)
+++ cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp Fri May 31 07:26:19 2019
@@ -517,6 +517,15 @@ void PR34109(int* a) {
   delete a;
 }
 
+namespace PR42089 {
+  struct S {
+__attribute__((nothrow)) void Foo(); // expected-note {{previous 
declaration is here}}
+__attribute__((nothrow)) void Bar();
+  };
+  void S::Foo(){} // expected-warning {{is missing exception specification}}
+  __attribute__((nothrow)) void S::Bar(){}
+}
+
 #elif TEST2
 
 // Check that __unaligned is not recognized if MS extensions are not enabled


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

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

[Keane, Erich via cfe-commits]

To Update: Nico and I discussed this on IRC and believe the best way to fix 
this is to simply suppress this when it comes from a macro-expansion in a 
system header ala r221172 (or r220992).

I intend to work on that fix next after my current (fairly short) fix.

Thanks!
-Erich

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Friday, May 31, 2019 6:58 AM
To: Keane, Erich 
Cc: Aaron Ballman ; cfe-commits 

Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

I'm not sure this is the best fix. From the patch description, it sounds like 
this is a Microsoft-specific change. So splitting this into a new diag group 
that then everyone using the Microsoft headers has to disable seems a bit 
roundabout – don't we get the same behavior by not emitting this warning in the 
first place, except that we don't require everyone using clang-cl to explicitly 
disable this warning then?

On Fri, May 31, 2019 at 9:38 AM Keane, Erich 
mailto:erich.ke...@intel.com>> wrote:
Presumably the right choice for you on this one is to separate the diagnostic 
into a new group.  Copying Aaron since he might have an idea of an existing 
group, otherwise I’ll push a commit to put it in a new one.

Thanks for the report!
-Erich

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Thursday, May 30, 2019 6:15 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

Hello,

this causes this diagnostic when building on Windows:

In file included from 
../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10:
../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3): 
error: 'nothrow' attribute conflicts with exception specification; attribute 
ignored [-Werror,-Wignored-attributes]
  END_COM_MAP()
  ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2):
 note: expanded from macro 'END_COM_MAP'
STDMETHOD(QueryInterface)( \
^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42):
 note: expanded from macro 'STDMETHOD'
#define STDMETHOD(method)virtual COM_DECLSPEC_NOTHROW HRESULT 
STDMETHODCALLTYPE method
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30):
 note: expanded from macro 'COM_DECLSPEC_NOTHROW'
#define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39):
 note: expanded from macro 'DECLSPEC_NOTHROW'
#define DECLSPEC_NOTHROW   __declspec(nothrow)
  ^


Suggestions? We don't want to use -Wno-ignored-attributes since that disables 
this warning for all attributes.

All the involved macros are in system headers and we have no control over them.

On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Thu May 30 10:31:54 2019
New Revision: 362119

URL: http://llvm.org/viewvc/llvm-project?rev=362119=rev
Log:
Add Attribute NoThrow as an Exception Specifier Type

In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became
clear that the current mechanism of hacking through checks for the
exception specification of a function gets confused really quickly when
there are alternate exception specifiers.

This patch introcues EST_NoThrow, which is the equivilent of
EST_noexcept when caused by EST_noThrow. The existing implementation is
left in place to cover functions with no FunctionProtoType.

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

Added:
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp   (with props)
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=362119=362118=362119=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Thu May 30 10:31:54 2019
@@ 

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

[Keane, Erich via cfe-commits]

It IS an attribute that works on Linux as well (about ½ way down this page: 
https://gcc.gnu.org/onlinedocs/gcc-4.7.2/gcc/Function-Attributes.html), though 
used rarely (at least in C++ code) as far as I understand.

The fact that a mistake like this is in the ATL headers is frustrating, but I’m 
not sure the best way to deal with this.  Aaron and I both believed the warning 
to be meaningful/useful (since this conflict is the same as saying throw() 
noexcept(false)), so I’m not sure removing it is a good idea (if that’s what 
you’re suggesting).

However, if this is something reasonably common on Microsoft implementations, I 
wonder if it is a warning we can make off-by-default on clang-cl.  It would be 
a shame to lose it, but if this is a commonly enough used problem we might be 
stuck with it.

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Friday, May 31, 2019 6:58 AM
To: Keane, Erich 
Cc: Aaron Ballman ; cfe-commits 

Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

I'm not sure this is the best fix. From the patch description, it sounds like 
this is a Microsoft-specific change. So splitting this into a new diag group 
that then everyone using the Microsoft headers has to disable seems a bit 
roundabout – don't we get the same behavior by not emitting this warning in the 
first place, except that we don't require everyone using clang-cl to explicitly 
disable this warning then?

On Fri, May 31, 2019 at 9:38 AM Keane, Erich 
mailto:erich.ke...@intel.com>> wrote:
Presumably the right choice for you on this one is to separate the diagnostic 
into a new group.  Copying Aaron since he might have an idea of an existing 
group, otherwise I’ll push a commit to put it in a new one.

Thanks for the report!
-Erich

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Thursday, May 30, 2019 6:15 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

Hello,

this causes this diagnostic when building on Windows:

In file included from 
../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10:
../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3): 
error: 'nothrow' attribute conflicts with exception specification; attribute 
ignored [-Werror,-Wignored-attributes]
  END_COM_MAP()
  ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2):
 note: expanded from macro 'END_COM_MAP'
STDMETHOD(QueryInterface)( \
^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42):
 note: expanded from macro 'STDMETHOD'
#define STDMETHOD(method)virtual COM_DECLSPEC_NOTHROW HRESULT 
STDMETHODCALLTYPE method
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30):
 note: expanded from macro 'COM_DECLSPEC_NOTHROW'
#define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39):
 note: expanded from macro 'DECLSPEC_NOTHROW'
#define DECLSPEC_NOTHROW   __declspec(nothrow)
  ^


Suggestions? We don't want to use -Wno-ignored-attributes since that disables 
this warning for all attributes.

All the involved macros are in system headers and we have no control over them.

On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Thu May 30 10:31:54 2019
New Revision: 362119

URL: http://llvm.org/viewvc/llvm-project?rev=362119=rev
Log:
Add Attribute NoThrow as an Exception Specifier Type

In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became
clear that the current mechanism of hacking through checks for the
exception specification of a function gets confused really quickly when
there are alternate exception specifiers.

This patch introcues EST_NoThrow, which is the equivilent of
EST_noexcept when caused by EST_noThrow. The existing implementation is
left in place to cover functions with no FunctionProtoType.

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

Added:
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp   (with props)
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/lib/AST/Type.cpp

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added a reviewer: chandlerc.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Intel would like to contribute a patch to implement support for these Intel- 
and Microsoft -fp options.  This message is to describe the options and request 
feedback from the community.
-fp-model=[precise|strict|fast|except[-]] and -fp-speculation=[fast|strict|safe]

This contribution would dovetail with the llvm patch "Teach the IRBuilder about 
constrained fadd and friends" which is under review here, 
https://reviews.llvm.org/D53157/new/.  I have a patch ready to review that 
works with D53157 .  The motivation for 
providing these is that having a single option to control most basic FP options 
is better and easier to understand for users.

The companion llvm patch is available for review here: 
https://reviews.llvm.org/D62730 ; I made a couple small changes to D53157 
.

The option settings -fp-model=[precise|strict|fast|except] are supported by 
both ICC and CL. The fp-speculation option is supported only by ICC.  The CL 
and ICC -fp-model option is documented on these pages:

  
https://docs.microsoft.com/en-us/cpp/build/reference/fp-specify-floating-point-behavior?view=vs-2019
  
https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-fp-model-fp

Currently, clang's default behavior corresponds to -fp-model=precise.  
Clang/llvm support for -fp-model=[strict|except] is being developed in the 
D53157  patch, and there is current llvm 
support for the fast settings by using the fast math flags llvm::FastMathFlags. 
 Note: the clang-cl wrapper to support Microsoft options has simplified support 
for these options by mapping /fp-model=except to ftrapping-math, fp-mdel=fast 
to ffast-math, fp-model=precise and fp-model=strict to fno-fast-math (see 
clang/Driver/CLCompatOptions.td).

According to the online options documentation, you can combine except[-] with 
[precise|strict|fast], but combining both fast and except is not a valid 
setting (the Driver will emit an error).

  precise - Disables optimizations that are not value-safe on floating-point 
data, although FP contraction is enabled.
  strict - Enables precise and except, disables contractions (FMA), and enables 
pragma stdc fenv_access.   
  fast - Equivalent to -ffast-math
  except/except- - Determines whether strict floating-point exception semantics 
are honored.

The ICC option -fp-speculation is described here,

  
https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-fp-speculation-qfp-speculation

These are the meanings of the fp-speculation settings:

  fast - Tells the compiler to speculate on floating-point operations.  This is 
equivalent to “fpexcept.ignore” in the constrained intrinsics review D53157.
  strict - Tells the compiler to disable speculation on floating-point 
operations.  This is equivalent to “fpexcept.strict” in the constrained 
intrinsics review D53157.
  safe - Tells the compiler to disable speculation if there is a possibility 
that the speculation may cause a floating-point exception.  This is equivalent 
to “fpexcept.maytrap” in the constrained intrinsics review D53157.


Repository:
  rL LLVM

https://reviews.llvm.org/D62731

Files:
  include/clang/Basic/LangOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  lib/CodeGen/CodeGenFunction.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/fpconstrained.c
  test/Driver/clang_f_opts.c

Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -217,6 +217,11 @@
 // RUN: %clang -### -S -fexec-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
 // CHECK-INVALID-INPUT-CHARSET: error: invalid value 'iso-8859-1' in '-fexec-charset=iso-8859-1'
 
+// RUN: %clang -### -S -fp-model=fast -fp-model=except -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-FAST-EXCEPT %s
+// RUN: %clang -### -S -fp-model=fast -fp-model=except -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-FAST-EXCEPT %s
+// CHECK-INVALID-FAST-EXCEPT: error: invalid argument 'fp-model=fast' not allowed with 'fp-model=except'
+//
+
 // Test that we don't error on these.
 // RUN: %clang -### -S -Werror\
 // RUN: -falign-functions -falign-functions=2 -fno-align-functions\
Index: test/CodeGen/fpconstrained.c
===
--- /dev/null
+++ test/CodeGen/fpconstrained.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fp-model=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=STRICT
+// RUN: %clang_cc1 -fp-model=strict -fp-model-except -emit-llvm -o - %s | FileCheck %s 

Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

[Nico Weber via cfe-commits]

I'm not sure this is the best fix. From the patch description, it sounds
like this is a Microsoft-specific change. So splitting this into a new
diag group that then everyone using the Microsoft headers has to disable
seems a bit roundabout – don't we get the same behavior by not emitting
this warning in the first place, except that we don't require everyone
using clang-cl to explicitly disable this warning then?

On Fri, May 31, 2019 at 9:38 AM Keane, Erich  wrote:

> Presumably the right choice for you on this one is to separate the
> diagnostic into a new group.  Copying Aaron since he might have an idea of
> an existing group, otherwise I’ll push a commit to put it in a new one.
>
>
>
> Thanks for the report!
>
> -Erich
>
>
>
> *From:* Nico Weber [mailto:tha...@chromium.org]
> *Sent:* Thursday, May 30, 2019 6:15 PM
> *To:* Keane, Erich 
> *Cc:* cfe-commits 
> *Subject:* Re: r362119 - Add Attribute NoThrow as an Exception Specifier
> Type
>
>
>
> Hello,
>
>
>
> this causes this diagnostic when building on Windows:
>
>
>
> In file included from
> ../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10:
> ../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3):
> error: 'nothrow' attribute conflicts with exception specification;
> attribute ignored [-Werror,-Wignored-attributes]
>   END_COM_MAP()
>   ^
> ..\..\third_party\depot_tools\win_toolchain\vs_files\
> 818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2):
> note: expanded from macro 'END_COM_MAP'
> STDMETHOD(QueryInterface)( \
> ^
> ..\..\third_party\depot_tools\win_toolchain\vs_files\
> 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42):
> note: expanded from macro 'STDMETHOD'
> #define STDMETHOD(method)virtual COM_DECLSPEC_NOTHROW HRESULT
> STDMETHODCALLTYPE method
>  ^
> ..\..\third_party\depot_tools\win_toolchain\vs_files\
> 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30):
> note: expanded from macro 'COM_DECLSPEC_NOTHROW'
> #define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW
>  ^
> ..\..\third_party\depot_tools\win_toolchain\vs_files\
> 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39):
> note: expanded from macro 'DECLSPEC_NOTHROW'
> #define DECLSPEC_NOTHROW   __declspec(nothrow)
>   ^
>
>
>
>
>
> Suggestions? We don't want to use -Wno-ignored-attributes since that
> disables this warning for all attributes.
>
>
>
> All the involved macros are in system headers and we have no control over
> them.
>
>
>
> On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: erichkeane
> Date: Thu May 30 10:31:54 2019
> New Revision: 362119
>
> URL: http://llvm.org/viewvc/llvm-project?rev=362119=rev
> Log:
> Add Attribute NoThrow as an Exception Specifier Type
>
> In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became
> clear that the current mechanism of hacking through checks for the
> exception specification of a function gets confused really quickly when
> there are alternate exception specifiers.
>
> This patch introcues EST_NoThrow, which is the equivilent of
> EST_noexcept when caused by EST_noThrow. The existing implementation is
> left in place to cover functions with no FunctionProtoType.
>
> Differential Revision: https://reviews.llvm.org/D62435
>
> Added:
> cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp   (with props)
> Modified:
> cfe/trunk/include/clang-c/Index.h
> cfe/trunk/include/clang/AST/Decl.h
> cfe/trunk/include/clang/AST/Type.h
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/AST/JSONNodeDumper.cpp
> cfe/trunk/lib/AST/Type.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/lib/Sema/SemaType.cpp
> cfe/trunk/tools/libclang/CXType.cpp
>
> Modified: cfe/trunk/include/clang-c/Index.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=362119=362118=362119=diff
>
> ==
> --- cfe/trunk/include/clang-c/Index.h (original)
> +++ cfe/trunk/include/clang-c/Index.h Thu May 30 10:31:54 2019
> @@ -32,7 +32,7 @@
>   * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
>   */
>  #define CINDEX_VERSION_MAJOR 0
> -#define CINDEX_VERSION_MINOR 57
> +#define CINDEX_VERSION_MINOR 58
>
>  #define CINDEX_VERSION_ENCODE(major, minor) ( \
>((major) * 1)   \
> @@ -221,7 +221,12 @@ enum CXCursor_ExceptionSpecificationKind
>/**
> * The exception 

[PATCH] D62730: [RFC] Alternate implementation of D53157 IRBuilder for Constrained FP using enumeration vs MDNode and add support for fp-model and fp-speculation language options

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added reviewers: andrew.w.kaylor, kpn.
Herald added a subscriber: mgorny.
Herald added a project: LLVM.

This is part of RFC to support language options -fp-model and -fp-speculation.

This makes a small modification to D53157  to 
pass some arguments using an enumeration type instead of MDNode.

In addition I created the FPState.h file which declares 2 enumeration types 
corresponding to the option settings.  I want to put them into llvm so it will 
be available not only to clang but to other language front ends that might want 
to offer floating point control using option settings.  The language options 
are combined using certain semantics into IRBuilder settings.  There's a member 
function in FPState.h that does this.

There's a clang patch that goes along with this patch.

Here's the RFC pasted from email to llvm-dev and clang-dev: 
Intel would like to contribute a patch to implement support for these Intel- 
and Microsoft -fp options:
-fp-model=[precise|strict|fast|except[-]] and -fp-speculation=[fast|strict|safe]

This contribution would dovetail with the llvm patch "Teach the IRBuilder about 
constrained fadd and friends" which is under review here, 
https://reviews.llvm.org/D53157/new/.  I have a patch ready to review that 
works with D53157 .  The motivation for 
providing these is that having a single option to control most basic FP options 
is better and easier to understand for users.

The option settings -fp-model=[precise|strict|fast|except] are supported by 
both ICC and CL. The fp-speculation option is supported only by ICC.  The CL 
and ICC -fp-model option is documented on these pages:

  
https://docs.microsoft.com/en-us/cpp/build/reference/fp-specify-floating-point-behavior?view=vs-2019
  
https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-fp-model-fp

Currently, clang's default behavior corresponds to -fp-model=precise.  
Clang/llvm support for -fp-model=[strict|except] is being developed in the 
D53157  patch, and there is current llvm 
support for the fast settings by using the fast math flags llvm::FastMathFlags. 
 Note: the clang-cl wrapper to support Microsoft options has simplified support 
for these options by mapping /fp-model=except to ftrapping-math, fp-mdel=fast 
to ffast-math, fp-model=precise and fp-model=strict to fno-fast-math (see 
clang/Driver/CLCompatOptions.td).

According to the online options documentation, you can combine except[-] with 
[precise|strict|fast], but combining both fast and except is not a valid 
setting (the Driver will emit an error).

  precise - Disables optimizations that are not value-safe on floating-point 
data, although FP contraction is enabled.
  strict - Enables precise and except, disables contractions (FMA), and enables 
pragma stdc fenv_access.   
  fast - Equivalent to -ffast-math
  except/except- - Determines whether strict floating-point exception semantics 
are honored.

The ICC option -fp-speculation is described here,

  
https://software.intel.com/en-us/cpp-compiler-developer-guide-and-reference-fp-speculation-qfp-speculation

These are the meanings of the fp-speculation settings:

  fast - Tells the compiler to speculate on floating-point operations.  This is 
equivalent to “fpexcept.ignore” in the constrained intrinsics review D53157.
  strict - Tells the compiler to disable speculation on floating-point 
operations.  This is equivalent to “fpexcept.strict” in the constrained 
intrinsics review D53157.
  safe - Tells the compiler to disable speculation if there is a possibility 
that the speculation may cause a floating-point exception.  This is equivalent 
to “fpexcept.maytrap” in the constrained intrinsics review D53157.


Repository:
  rL LLVM

https://reviews.llvm.org/D62730

Files:
  include/llvm/IR/FPState.h
  include/llvm/IR/IRBuilder.h
  lib/IR/CMakeLists.txt
  lib/IR/FPState.cpp
  unittests/IR/IRBuilderTest.cpp

Index: unittests/IR/IRBuilderTest.cpp
===
--- unittests/IR/IRBuilderTest.cpp
+++ unittests/IR/IRBuilderTest.cpp
@@ -10,6 +10,7 @@
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/FPState.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
@@ -122,6 +123,159 @@
   EXPECT_FALSE(II->hasNoNaNs());
 }
 
+TEST_F(IRBuilderTest, ConstrainedFP) {
+  IRBuilder<> Builder(BB);
+  Value *V;
+  CallInst *Call;
+  IntrinsicInst *II;
+
+  V = Builder.CreateLoad(GV);
+
+  Call = Builder.CreateConstrainedFAdd(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+
+  Call = Builder.CreateConstrainedFSub(V, V);
+  II = cast(Call);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fsub);
+
+  Call = 

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added a comment.

Ah yes, the school boy error! ;-) Actually, there was a test, but in a 
different patch; I will move it to here.


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

https://reviews.llvm.org/D60697



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

RE: r362119 - Add Attribute NoThrow as an Exception Specifier Type

[Keane, Erich via cfe-commits]

Presumably the right choice for you on this one is to separate the diagnostic 
into a new group.  Copying Aaron since he might have an idea of an existing 
group, otherwise I’ll push a commit to put it in a new one.

Thanks for the report!
-Erich

From: Nico Weber [mailto:tha...@chromium.org]
Sent: Thursday, May 30, 2019 6:15 PM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type

Hello,

this causes this diagnostic when building on Windows:

In file included from 
../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10:
../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3): 
error: 'nothrow' attribute conflicts with exception specification; attribute 
ignored [-Werror,-Wignored-attributes]
  END_COM_MAP()
  ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2):
 note: expanded from macro 'END_COM_MAP'
STDMETHOD(QueryInterface)( \
^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42):
 note: expanded from macro 'STDMETHOD'
#define STDMETHOD(method)virtual COM_DECLSPEC_NOTHROW HRESULT 
STDMETHODCALLTYPE method
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30):
 note: expanded from macro 'COM_DECLSPEC_NOTHROW'
#define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW
 ^
..\..\third_party\depot_tools\win_toolchain\vs_files\ 
818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39):
 note: expanded from macro 'DECLSPEC_NOTHROW'
#define DECLSPEC_NOTHROW   __declspec(nothrow)
  ^


Suggestions? We don't want to use -Wno-ignored-attributes since that disables 
this warning for all attributes.

All the involved macros are in system headers and we have no control over them.

On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Thu May 30 10:31:54 2019
New Revision: 362119

URL: http://llvm.org/viewvc/llvm-project?rev=362119=rev
Log:
Add Attribute NoThrow as an Exception Specifier Type

In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became
clear that the current mechanism of hacking through checks for the
exception specification of a function gets confused really quickly when
there are alternate exception specifiers.

This patch introcues EST_NoThrow, which is the equivilent of
EST_noexcept when caused by EST_noThrow. The existing implementation is
left in place to cover functions with no FunctionProtoType.

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

Added:
cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp   (with props)
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=362119=362118=362119=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Thu May 30 10:31:54 2019
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 57
+#define CINDEX_VERSION_MINOR 58

 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -221,7 +221,12 @@ enum CXCursor_ExceptionSpecificationKind
   /**
* The exception specification has not been parsed yet.
*/
-  CXCursor_ExceptionSpecificationKind_Unparsed
+  CXCursor_ExceptionSpecificationKind_Unparsed,
+
+  /**
+   * The cursor has a __declspec(nothrow) exception specification.
+   */
+  CXCursor_ExceptionSpecificationKind_NoThrow
 };

 /**

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=362119=362118=362119=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Thu May 30 10:31:54 2019
@@ -2330,6 +2330,14 @@ public:
 return T->castAs()->getReturnType();
   }

+  

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

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

This still needs tests adding.


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

https://reviews.llvm.org/D60697



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

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D62435#1524824 , @mstorsjo wrote:

> This change broke compiling Qt on MinGW, see 
> https://bugs.llvm.org/show_bug.cgi?id=42089. Trivially reproducible by trying 
> to compile a snippet that looks like this:
>
>   class Foo {
>   public:
>   __attribute__((nothrow)) void __attribute__((__stdcall__)) Bar();
>   };
>  
>   void Foo::Bar() {
>   }
>
>
> Reproducible by compiling the same snippet for a windows-msvc target as well.


Yikes, thanks for the report!  I'll look at it this morning.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62435



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