[PATCH] D39903: [libclang] Allow pretty printing declarations

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

> It might be worth adding some very simple get/set tests to ensure that 
> properties are set as intended.

clang_PrintingPolicy_setProperty is already called in c-index-test.c and 
covered with test/Index/print-display-names.cpp. Do you have another kind of 
test in mind?

I'm not sure how to best test clang_PrintingPolicy_getProperty. I don't see how 
to get that cleanly covered within c-index-test.c, too. After calling the 
setter, one could call the getter and verify that it's the same. But 
c-index-test shouldn't do something like this. I've added a test in 
LibclangTest.cpp with which I'm not too happy (an extra parse just to test a 
getter feels wrong).




Comment at: tools/libclang/CIndex.cpp:4720
+
+#define HANDLE_CASE(P, PROPERTY_NAME)  
\
+  case CXPrintingPolicy_##PROPERTY_NAME:   
\

jbcoe wrote:
> I’m not convinced that the code replaced by the macro is sufficiently 
> complicated to justify use of a macro.
I find it easier to read and verify (mapping to the wrong property can't 
happen), but I've no strong opinion here, fine with me - reverted to non-macro 
case.



Comment at: tools/libclang/libclang.exports:365
+clang_PrintingPolicy_get
+clang_PrintingPolicy_set
+clang_PrintingPolicy_dispose

jbcoe wrote:
> clang_PrintingPolicy_setProperty and clang_PrintingPolicy_getProperty might 
> be better names (I know the ones you have used were my earlier suggestions).
Agree!


Repository:
  rC Clang

https://reviews.llvm.org/D39903



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

[PATCH] D39903: [libclang] Allow pretty printing declarations

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 129591.
nik added a comment.

Addressed comments.


Repository:
  rC Clang

https://reviews.llvm.org/D39903

Files:
  include/clang-c/Index.h
  test/Index/print-display-names.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports
  unittests/libclang/LibclangTest.cpp

Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -572,3 +572,26 @@
   EXPECT_EQ(0U, clang_getNumDiagnostics(ClangTU));
   DisplayDiagnostics();
 }
+
+class LibclangPrintingPolicyTest : public LibclangParseTest {
+public:
+  CXPrintingPolicy Policy = nullptr;
+
+  void SetUp() override {
+LibclangParseTest::SetUp();
+std::string File = "file.cpp";
+WriteFile(File, "int i;\n");
+ClangTU = clang_parseTranslationUnit(Index, File.c_str(), nullptr, 0,
+ nullptr, 0, TUFlags);
+CXCursor TUCursor = clang_getTranslationUnitCursor(ClangTU);
+Policy = clang_getCursorPrintingPolicy(TUCursor);
+  }
+  void TearDown() override {
+clang_PrintingPolicy_dispose(Policy);
+LibclangParseTest::TearDown();
+  }
+};
+
+TEST_F(LibclangPrintingPolicyTest, GetProperty) {
+  EXPECT_EQ(2U, clang_PrintingPolicy_getProperty(Policy, CXPrintingPolicy_Indentation));
+}
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -178,6 +178,8 @@
 clang_getCursorCompletionString
 clang_getCursorDefinition
 clang_getCursorDisplayName
+clang_getCursorPrintingPolicy
+clang_getCursorPrettyPrinted
 clang_getCursorExtent
 clang_getCursorExceptionSpecificationType
 clang_getCursorKind
@@ -359,3 +361,6 @@
 clang_EvalResult_getAsDouble
 clang_EvalResult_getAsStr
 clang_EvalResult_dispose
+clang_PrintingPolicy_getProperty
+clang_PrintingPolicy_setProperty
+clang_PrintingPolicy_dispose
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4706,6 +4706,192 @@
   return cxstring::createSet(Manglings);
 }
 
+CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) {
+  if (clang_Cursor_isNull(C))
+return 0;
+  return new PrintingPolicy(getCursorContext(C).getPrintingPolicy());
+}
+
+void clang_PrintingPolicy_dispose(CXPrintingPolicy Policy) {
+  if (Policy)
+delete static_cast(Policy);
+}
+
+unsigned
+clang_PrintingPolicy_getProperty(CXPrintingPolicy Policy,
+ enum CXPrintingPolicyProperty Property) {
+  if (!Policy)
+return 0;
+
+  PrintingPolicy *P = static_cast(Policy);
+  switch (Property) {
+  case CXPrintingPolicy_Indentation:
+return P->Indentation;
+  case CXPrintingPolicy_SuppressSpecifiers:
+return P->SuppressSpecifiers;
+  case CXPrintingPolicy_SuppressTagKeyword:
+return P->SuppressTagKeyword;
+  case CXPrintingPolicy_IncludeTagDefinition:
+return P->IncludeTagDefinition;
+  case CXPrintingPolicy_SuppressScope:
+return P->SuppressScope;
+  case CXPrintingPolicy_SuppressUnwrittenScope:
+return P->SuppressUnwrittenScope;
+  case CXPrintingPolicy_SuppressInitializers:
+return P->SuppressInitializers;
+  case CXPrintingPolicy_ConstantArraySizeAsWritten:
+return P->ConstantArraySizeAsWritten;
+  case CXPrintingPolicy_AnonymousTagLocations:
+return P->AnonymousTagLocations;
+  case CXPrintingPolicy_SuppressStrongLifetime:
+return P->SuppressStrongLifetime;
+  case CXPrintingPolicy_SuppressLifetimeQualifiers:
+return P->SuppressLifetimeQualifiers;
+  case CXPrintingPolicy_SuppressTemplateArgsInCXXConstructors:
+return P->SuppressTemplateArgsInCXXConstructors;
+  case CXPrintingPolicy_Bool:
+return P->Bool;
+  case CXPrintingPolicy_Restrict:
+return P->Restrict;
+  case CXPrintingPolicy_Alignof:
+return P->Alignof;
+  case CXPrintingPolicy_UnderscoreAlignof:
+return P->UnderscoreAlignof;
+  case CXPrintingPolicy_UseVoidForZeroParams:
+return P->UseVoidForZeroParams;
+  case CXPrintingPolicy_TerseOutput:
+return P->TerseOutput;
+  case CXPrintingPolicy_PolishForDeclaration:
+return P->PolishForDeclaration;
+  case CXPrintingPolicy_Half:
+return P->Half;
+  case CXPrintingPolicy_MSWChar:
+return P->MSWChar;
+  case CXPrintingPolicy_IncludeNewlines:
+return P->IncludeNewlines;
+  case CXPrintingPolicy_MSVCFormatting:
+return P->MSVCFormatting;
+  case CXPrintingPolicy_ConstantsAsWritten:
+return P->ConstantsAsWritten;
+  case CXPrintingPolicy_SuppressImplicitBase:
+return P->SuppressImplicitBase;
+  case CXPrintingPolicy_FullyQualifiedName:
+return P->FullyQualifiedName;
+  }
+
+  return 0;
+}
+
+void clang_PrintingPolicy_setProperty(CXPrintingPolicy Policy,
+  enum CXPrintingPolicyPrope

[PATCH] D41897: Fixing a crash in Sema.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added inline comments.



Comment at: Sema/SemaDeclCXX.cpp:2426
+// Skip all dependent types in templates being used as base specifiers.
+// Checks below assume that base specifier is a CXXRecord.
+if (BaseType->isDependentType()) {

aaron.ballman wrote:
> that base -> that the base
Good catch. Thanks.



Comment at: Sema/SemaDeclCXX.cpp:2427-2429
+if (BaseType->isDependentType()) {
+  continue;
+}

aaron.ballman wrote:
> You can elide the braces here.
Of course. Thanks for reminder.



Comment at: SemaCXX/base-class-ambiguity-check.cpp:1
+// RUN: %clang_cc1 -fsyntax-only %s
+

aaron.ballman wrote:
> This run line isn't testing anything. Since you're trying to ensure this 
> doesn't crash, I would put `-verify` on the RUN line and `// 
> expected-no-diagnostics` on the line below.
I know what you mean. I was worried about that as well so I asked other guys. 
Apparently this is enough. If you run the test on master this is what you get:


```
> bin/llvm-lit 
> /Users/jankorous/src/oss/llvm/llvm/tools/clang/test/SemaCXX/base-class-ambiguity-check.cpp
llvm-lit: /Users/jankorous/src/oss/llvm/llvm/utils/lit/lit/llvm/config.py:334: 
note: using clang: /Users/jankorous/src/oss/llvm/build/bin/clang
llvm-lit: /Users/jankorous/src/oss/llvm/llvm/utils/lit/lit/util.py:379: note: 
using SDKROOT: 
'/Applications/Edge9E77_AllSDKs_fromNFA/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk'
-- Testing: 1 tests, 1 threads --
FAIL: Clang :: SemaCXX/base-class-ambiguity-check.cpp (1 of 1)
Testing Time: 0.31s

Failing Tests (1):
Clang :: SemaCXX/base-class-ambiguity-check.cpp

  Unexpected Failures: 1

```



Comment at: SemaCXX/base-class-ambiguity-check.cpp:1
+// RUN: %clang_cc1 -fsyntax-only %s
+

aaron.ballman wrote:
> jkorous-apple wrote:
> > aaron.ballman wrote:
> > > This run line isn't testing anything. Since you're trying to ensure this 
> > > doesn't crash, I would put `-verify` on the RUN line and `// 
> > > expected-no-diagnostics` on the line below.
> > I know what you mean. I was worried about that as well so I asked other 
> > guys. Apparently this is enough. If you run the test on master this is what 
> > you get:
> > 
> > 
> > ```
> > > bin/llvm-lit 
> > > /Users/jankorous/src/oss/llvm/llvm/tools/clang/test/SemaCXX/base-class-ambiguity-check.cpp
> > llvm-lit: 
> > /Users/jankorous/src/oss/llvm/llvm/utils/lit/lit/llvm/config.py:334: note: 
> > using clang: /Users/jankorous/src/oss/llvm/build/bin/clang
> > llvm-lit: /Users/jankorous/src/oss/llvm/llvm/utils/lit/lit/util.py:379: 
> > note: using SDKROOT: 
> > '/Applications/Edge9E77_AllSDKs_fromNFA/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk'
> > -- Testing: 1 tests, 1 threads --
> > FAIL: Clang :: SemaCXX/base-class-ambiguity-check.cpp (1 of 1)
> > Testing Time: 0.31s
> > 
> > Failing Tests (1):
> > Clang :: SemaCXX/base-class-ambiguity-check.cpp
> > 
> >   Unexpected Failures: 1
> > 
> > ```
> This comment hasn't been applied yet.
Sorry, what do you mean?



Comment at: SemaCXX/base-class-ambiguity-check.cpp:9
+
+struct Derived : Base, T
+{ };

aaron.ballman wrote:
> I would add a comment around here explaining that this used to crash.
Good idea.



Comment at: SemaCXX/base-class-ambiguity-check.cpp:12
+};
\ No newline at end of file


aaron.ballman wrote:
> Can you add a newline at the end of the file, and then run the file through 
> clang-format to properly format it?
Sure. I forgot to clang-format the test.


https://reviews.llvm.org/D41897



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

[PATCH] D39903: [libclang] Allow pretty printing declarations

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

Looking good, only a few nits.




Comment at: tools/libclang/CIndex.cpp:4782
+
+  return 0;
+}

Might be worth asserting here.



Comment at: unittests/libclang/LibclangTest.cpp:596
+TEST_F(LibclangPrintingPolicyTest, GetProperty) {
+  EXPECT_EQ(2U, clang_PrintingPolicy_getProperty(Policy, 
CXPrintingPolicy_Indentation));
+}

It would be useful, albeit tedious, to add get/set test pairs for each property.


Repository:
  rC Clang

https://reviews.llvm.org/D39903



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

[PATCH] D39903: [libclang] Allow pretty printing declarations

[Nikolai Kosjar via Phabricator via cfe-commits]

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



Comment at: tools/libclang/CIndex.cpp:4782
+
+  return 0;
+}

jbcoe wrote:
> Might be worth asserting here.
Good idea. I've done the same for the setter.



Comment at: unittests/libclang/LibclangTest.cpp:596
+TEST_F(LibclangPrintingPolicyTest, GetProperty) {
+  EXPECT_EQ(2U, clang_PrintingPolicy_getProperty(Policy, 
CXPrintingPolicy_Indentation));
+}

jbcoe wrote:
> It would be useful, albeit tedious, to add get/set test pairs for each 
> property.
I think we have the basic functionality of the getter and setter covered. 
Testing each getter/setter for each property would mostly help to verify the 
mapping in the getters/setters - initially I've ruled out that possibility with 
the macros (once the macro is correct - but that's much easier to check). I 
would prefer to go back to the macro version than adding  a test here that goes 
over all properties.


Repository:
  rC Clang

https://reviews.llvm.org/D39903



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

[PATCH] D39903: [libclang] Allow pretty printing declarations

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 129600.
nik marked an inline comment as done.
nik added a comment.

Added assert() for getter/setter.


Repository:
  rC Clang

https://reviews.llvm.org/D39903

Files:
  include/clang-c/Index.h
  test/Index/print-display-names.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports
  unittests/libclang/LibclangTest.cpp

Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -572,3 +572,26 @@
   EXPECT_EQ(0U, clang_getNumDiagnostics(ClangTU));
   DisplayDiagnostics();
 }
+
+class LibclangPrintingPolicyTest : public LibclangParseTest {
+public:
+  CXPrintingPolicy Policy = nullptr;
+
+  void SetUp() override {
+LibclangParseTest::SetUp();
+std::string File = "file.cpp";
+WriteFile(File, "int i;\n");
+ClangTU = clang_parseTranslationUnit(Index, File.c_str(), nullptr, 0,
+ nullptr, 0, TUFlags);
+CXCursor TUCursor = clang_getTranslationUnitCursor(ClangTU);
+Policy = clang_getCursorPrintingPolicy(TUCursor);
+  }
+  void TearDown() override {
+clang_PrintingPolicy_dispose(Policy);
+LibclangParseTest::TearDown();
+  }
+};
+
+TEST_F(LibclangPrintingPolicyTest, GetProperty) {
+  EXPECT_EQ(2U, clang_PrintingPolicy_getProperty(Policy, CXPrintingPolicy_Indentation));
+}
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -178,6 +178,8 @@
 clang_getCursorCompletionString
 clang_getCursorDefinition
 clang_getCursorDisplayName
+clang_getCursorPrintingPolicy
+clang_getCursorPrettyPrinted
 clang_getCursorExtent
 clang_getCursorExceptionSpecificationType
 clang_getCursorKind
@@ -359,3 +361,6 @@
 clang_EvalResult_getAsDouble
 clang_EvalResult_getAsStr
 clang_EvalResult_dispose
+clang_PrintingPolicy_getProperty
+clang_PrintingPolicy_setProperty
+clang_PrintingPolicy_dispose
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4706,6 +4706,195 @@
   return cxstring::createSet(Manglings);
 }
 
+CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) {
+  if (clang_Cursor_isNull(C))
+return 0;
+  return new PrintingPolicy(getCursorContext(C).getPrintingPolicy());
+}
+
+void clang_PrintingPolicy_dispose(CXPrintingPolicy Policy) {
+  if (Policy)
+delete static_cast(Policy);
+}
+
+unsigned
+clang_PrintingPolicy_getProperty(CXPrintingPolicy Policy,
+ enum CXPrintingPolicyProperty Property) {
+  if (!Policy)
+return 0;
+
+  PrintingPolicy *P = static_cast(Policy);
+  switch (Property) {
+  case CXPrintingPolicy_Indentation:
+return P->Indentation;
+  case CXPrintingPolicy_SuppressSpecifiers:
+return P->SuppressSpecifiers;
+  case CXPrintingPolicy_SuppressTagKeyword:
+return P->SuppressTagKeyword;
+  case CXPrintingPolicy_IncludeTagDefinition:
+return P->IncludeTagDefinition;
+  case CXPrintingPolicy_SuppressScope:
+return P->SuppressScope;
+  case CXPrintingPolicy_SuppressUnwrittenScope:
+return P->SuppressUnwrittenScope;
+  case CXPrintingPolicy_SuppressInitializers:
+return P->SuppressInitializers;
+  case CXPrintingPolicy_ConstantArraySizeAsWritten:
+return P->ConstantArraySizeAsWritten;
+  case CXPrintingPolicy_AnonymousTagLocations:
+return P->AnonymousTagLocations;
+  case CXPrintingPolicy_SuppressStrongLifetime:
+return P->SuppressStrongLifetime;
+  case CXPrintingPolicy_SuppressLifetimeQualifiers:
+return P->SuppressLifetimeQualifiers;
+  case CXPrintingPolicy_SuppressTemplateArgsInCXXConstructors:
+return P->SuppressTemplateArgsInCXXConstructors;
+  case CXPrintingPolicy_Bool:
+return P->Bool;
+  case CXPrintingPolicy_Restrict:
+return P->Restrict;
+  case CXPrintingPolicy_Alignof:
+return P->Alignof;
+  case CXPrintingPolicy_UnderscoreAlignof:
+return P->UnderscoreAlignof;
+  case CXPrintingPolicy_UseVoidForZeroParams:
+return P->UseVoidForZeroParams;
+  case CXPrintingPolicy_TerseOutput:
+return P->TerseOutput;
+  case CXPrintingPolicy_PolishForDeclaration:
+return P->PolishForDeclaration;
+  case CXPrintingPolicy_Half:
+return P->Half;
+  case CXPrintingPolicy_MSWChar:
+return P->MSWChar;
+  case CXPrintingPolicy_IncludeNewlines:
+return P->IncludeNewlines;
+  case CXPrintingPolicy_MSVCFormatting:
+return P->MSVCFormatting;
+  case CXPrintingPolicy_ConstantsAsWritten:
+return P->ConstantsAsWritten;
+  case CXPrintingPolicy_SuppressImplicitBase:
+return P->SuppressImplicitBase;
+  case CXPrintingPolicy_FullyQualifiedName:
+return P->FullyQualifiedName;
+  }
+
+  assert(false && "Invalid CXPrintingPolicyProperty");
+  return 0;
+}
+
+void clang_PrintingP

[PATCH] D41989: [CodeComplete] Add an option to omit results from the preamble.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added reviewers: bkramer, ilya-biryukov.
Herald added a subscriber: cfe-commits.

Enumerating the contents of a namespace or global scope will omit any
decls that aren't already loaded, instead of deserializing them from the
PCH.

This allows a fast hybrid code completion where symbols from headers are
provided by an external index. (Sema already exposes the information
needed to do a reasonabl job of filtering them).
Clangd plans to implement this hybrid.

This option is just a hint - callers still need to postfilter results if
they want to *avoid* completing decls outside the main file.


Repository:
  rC Clang

https://reviews.llvm.org/D41989

Files:
  include/clang-c/Index.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/CodeCompleteOptions.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaLookup.cpp
  test/Index/complete-pch-skip.cpp
  test/Index/complete-pch-skip.h
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -643,6 +643,7 @@
   ArrayRef unsaved_files,
   unsigned options) {
   bool IncludeBriefComments = options & CXCodeComplete_IncludeBriefComments;
+  bool SkipPreamble = options & CXCodeComplete_SkipPreamble;
 
 #ifdef UDP_CODE_COMPLETION_LOGGER
 #ifdef UDP_CODE_COMPLETION_LOGGER_PORT
@@ -689,6 +690,7 @@
   // Create a code-completion consumer to capture the results.
   CodeCompleteOptions Opts;
   Opts.IncludeBriefComments = IncludeBriefComments;
+  Opts.LoadExternal = !SkipPreamble;
   CaptureCompletionResults Capture(Opts, *Results, &TU);
 
   // Perform completion.
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -2326,6 +2326,8 @@
 completionOptions |= CXCodeComplete_IncludeCodePatterns;
   if (getenv("CINDEXTEST_COMPLETION_BRIEF_COMMENTS"))
 completionOptions |= CXCodeComplete_IncludeBriefComments;
+  if (getenv("CINDEXTEST_COMPLETION_SKIP_PREAMBLE"))
+completionOptions |= CXCodeComplete_SkipPreamble;
   
   if (timing_only)
 input += strlen("-code-completion-timing=");
Index: test/Index/complete-pch-skip.h
===
--- /dev/null
+++ test/Index/complete-pch-skip.h
@@ -0,0 +1,3 @@
+namespace ns {
+int foo;
+}
Index: test/Index/complete-pch-skip.cpp
===
--- /dev/null
+++ test/Index/complete-pch-skip.cpp
@@ -0,0 +1,18 @@
+namespace ns {
+int bar;
+}
+
+int main() { return ns:: }
+
+// RUN: echo "namespace ns { int foo; }" > %t.h
+// RUN: c-index-test -write-pch %t.h.pch -x c++-header %t.h
+//
+// RUN: c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=WITH-PCH %s
+// WITH-PCH: {TypedText bar}
+// WITH-PCH: {TypedText foo}
+
+// RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH %s
+// SKIP-PCH-NOT: foo
+// SKIP-PCH: {TypedText bar}
+// SKIP-PCH-NOT: foo
+
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3498,7 +3498,8 @@
bool InBaseClass,
VisibleDeclConsumer &Consumer,
VisibleDeclsRecord &Visited,
-   bool IncludeDependentBases = false) {
+   bool IncludeDependentBases,
+   bool Load) {
   if (!Ctx)
 return;
 
@@ -3513,11 +3514,12 @@
 auto &Idents = S.Context.Idents;
 
 // Ensure all external identifiers are in the identifier table.
-if (IdentifierInfoLookup *External = Idents.getExternalIdentifierLookup()) {
-  std::unique_ptr Iter(External->getIdentifiers());
-  for (StringRef Name = Iter->Next(); !Name.empty(); Name = Iter->Next())
-Idents.get(Name);
-}
+if (Load)
+  if (IdentifierInfoLookup *External = Idents.getExternalIdentifierLookup()) {
+std::unique_ptr Iter(External->getIdentifiers());
+for (StringRef Name = Iter->Next(); !Name.empty(); Name = Iter->Next())
+  Idents.get(Name);
+  }
 
 // Walk all lookup results in the TU for each identifier.
 for (const auto &Ident : Idents) {
@@ -3540,7 +3542,8 @@
 Result.getSema().ForceDeclarationOfImplicitMembers(Class);
 
   // Enumerate all of the results in this context.
-  for (DeclContextLookupResult R : Ctx->lookups()) {
+  for (DeclContextLookupResult R :
+   Load ? Ctx->lookups() :

[PATCH] D41454: [clangd] Add ClangdUnit diagnostics tests using annotated code.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

ping... just realized this didn't go in


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41454



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[Alex Bradbury via Phabricator via cfe-commits]

asb updated this revision to Diff 129607.
asb marked 9 inline comments as done.
asb added a comment.
Herald added a subscriber: niosHD.

I've addressed all outstanding comments (a number of response are inline).

Additionally:

Floats in unions are always passed in GPRs (as clarified here 
 and here 
. However this patch is 
only concerned with the soft-float ABI, so the clarification doesn't directly 
affect us.

I've added tests for _Bool and empty structs and unions. GCC ignores empty 
structs and unions (it previously didn't mention unions but this PR 
 will clarify).

I've pinged the issue  
on the ABI documentation repo regarding bit fields, hopefully one of the GCC 
developers can comment to clarify what they're doing. I'd hope we can return to 
bitfields in a follow-on patch.


https://reviews.llvm.org/D40023

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/riscv32-abi.c
  test/CodeGen/riscv64-abi.c
  test/Driver/riscv32-toolchain.c
  test/Driver/riscv64-toolchain.c

Index: test/Driver/riscv64-toolchain.c
===
--- test/Driver/riscv64-toolchain.c
+++ test/Driver/riscv64-toolchain.c
@@ -42,3 +42,50 @@
 
 // CHECK: @align_vl = global i32 8
 int align_vl = __alignof(va_list);
+
+// Check types
+
+// CHECK: define zeroext i8 @check_char()
+char check_char() { return 0; }
+
+// CHECK: define signext i16 @check_short()
+short check_short() { return 0; }
+
+// CHECK: define signext i32 @check_int()
+int check_int() { return 0; }
+
+// CHECK: define signext i32 @check_wchar_t()
+int check_wchar_t() { return 0; }
+
+// CHECK: define i64 @check_long()
+long check_long() { return 0; }
+
+// CHECK: define i64 @check_longlong()
+long long check_longlong() { return 0; }
+
+// CHECK: define zeroext i8 @check_uchar()
+unsigned char check_uchar() { return 0; }
+
+// CHECK: define zeroext i16 @check_ushort()
+unsigned short check_ushort() { return 0; }
+
+// CHECK: define signext i32 @check_uint()
+unsigned int check_uint() { return 0; }
+
+// CHECK: define i64 @check_ulong()
+unsigned long check_ulong() { return 0; }
+
+// CHECK: define i64 @check_ulonglong()
+unsigned long long check_ulonglong() { return 0; }
+
+// CHECK: define i64 @check_size_t()
+size_t check_size_t() { return 0; }
+
+// CHECK: define float @check_float()
+float check_float() { return 0; }
+
+// CHECK: define double @check_double()
+double check_double() { return 0; }
+
+// CHECK: define fp128 @check_longdouble()
+long double check_longdouble() { return 0; }
Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -73,3 +73,50 @@
 
 // CHECK: @align_vl = global i32 4
 int align_vl = __alignof(va_list);
+
+// Check types
+
+// CHECK: zeroext i8 @check_char()
+char check_char() { return 0; }
+
+// CHECK: define signext i16 @check_short()
+short check_short() { return 0; }
+
+// CHECK: define i32 @check_int()
+int check_int() { return 0; }
+
+// CHECK: define i32 @check_wchar_t()
+int check_wchar_t() { return 0; }
+
+// CHECK: define i32 @check_long()
+long check_long() { return 0; }
+
+// CHECK: define i64 @check_longlong()
+long long check_longlong() { return 0; }
+
+// CHECK: define zeroext i8 @check_uchar()
+unsigned char check_uchar() { return 0; }
+
+// CHECK: define zeroext i16 @check_ushort()
+unsigned short check_ushort() { return 0; }
+
+// CHECK: define i32 @check_uint()
+unsigned int check_uint() { return 0; }
+
+// CHECK: define i32 @check_ulong()
+unsigned long check_ulong() { return 0; }
+
+// CHECK: define i64 @check_ulonglong()
+unsigned long long check_ulonglong() { return 0; }
+
+// CHECK: define i32 @check_size_t()
+size_t check_size_t() { return 0; }
+
+// CHECK: define float @check_float()
+float check_float() { return 0; }
+
+// CHECK: define double @check_double()
+double check_double() { return 0; }
+
+// CHECK: define fp128 @check_longdouble()
+long double check_longdouble() { return 0; }
Index: test/CodeGen/riscv64-abi.c
===
--- /dev/null
+++ test/CodeGen/riscv64-abi.c
@@ -0,0 +1,425 @@
+// RUN: %clang_cc1 -triple riscv64 -emit-llvm %s -o - | FileCheck %s
+
+#include 
+#include 
+
+// CHECK-LABEL: define void @f_void()
+void f_void(void) {}
+
+// Scalar arguments and return values smaller than the word size are extended
+// according to the sign of their type, up to 32 bits
+
+// CHECK-LABEL: define zeroext i1 @f_scalar_0(i1 zeroext %x)
+_Bool f_scalar_0(_Bool x) { return x; }
+
+// CHECK-LABEL: define signext i8 @f_scalar_1(i8 signext %x)
+int8_t f_scalar_1(int8_t x) { return x; }
+
+// CHECK-LABEL: define zeroext i8 @f_scalar_2(i

[PATCH] D40023: [RISCV] Implement ABI lowering

[Alex Bradbury via Phabricator via cfe-commits]

asb added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1937
+RetAttrs.addAttribute(llvm::Attribute::ZExt);
+}
 // FALL THROUGH

rjmccall wrote:
> I feel like a better design would be to record whether to do a sext or a zext 
> in the ABIArgInfo.  Add getSignExtend and getZeroExtend static functions to 
> ABIArgInfo and make getExtend a convenience function that takes a QualType 
> and uses its signedness.
I could see how that might be cleaner, but that's a larger refactoring that's 
going to touch a lot more code. Are you happy for this patch to stick with this 
more incremental change (applying the same sign-extension logic to return 
values as is used for arguments), and to leave your suggested refactoring for a 
future patch?



Comment at: lib/CodeGen/TargetInfo.cpp:8824
+
+  // We must track the number of GPRs used in order to conform to the RISC-V
+  // ABI, as integer scalars passed in registers should have signext/zeroext

mgrang wrote:
> I observed that you use RISCV and RISC-V interchangeably in comments. This is 
> not a problem,  per se but can make this uniform if we want to be *really* 
> particular about this :)
Slightly different contexts. 'RISC-V' is the proper name of the target but 
'RISCV' is the spelling of the LLVM target implementation. For this file at 
least, I think it makes sense.



Comment at: lib/CodeGen/TargetInfo.cpp:8858
+
+  uint64_t Size = getContext().getTypeSize(Ty);
+  uint64_t NeededAlign = getContext().getTypeAlign(Ty);

rjmccall wrote:
> I would encourage you to use CharUnits and getTypeSizeInChars more 
> consistently in this code; it would simplify some of the conversions you're 
> doing and eliminate some of the risk of forgetting a bits-to-bytes 
> conversion.  It looks like storing XLen as a CharUnits would also make this 
> easier.
XLen is defined throughout the RISC-V ISA and ABI documentation as the width of 
the integer ('x') registers in bits. I've modified EmitVAArg to make use of 
CharUnits to avoid a conversion - there don't seem to be any other bit/byte 
conversions I can see.



Comment at: lib/CodeGen/TargetInfo.cpp:8913
+  }
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+}

efriedma wrote:
> The spec says "Aggregates larger than 2✕XLEN bits are passed by reference and 
> are replaced in the argument list with the address".  That's not byval.
The LLVM backend lowers byval in that way. Given that at this point we have a 
trivial data type (plain struct or similar) which is copied-by-value by C 
semantics, the only difference is whether the generated IR indicates implicit 
copying with 'byval' or relies on the caller explicitly doing the copy. For 
some reason I thought 'byval' was preferred, but looking again it seems 
uncommon to do it this way. I've changed it to false - thanks for spotting this.


https://reviews.llvm.org/D40023



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

[PATCH] D39903: [libclang] Allow pretty printing declarations

[Jonathan B Coe via Phabricator via cfe-commits]

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



Comment at: unittests/libclang/LibclangTest.cpp:596
+TEST_F(LibclangPrintingPolicyTest, GetProperty) {
+  EXPECT_EQ(2U, clang_PrintingPolicy_getProperty(Policy, 
CXPrintingPolicy_Indentation));
+}

nik wrote:
> jbcoe wrote:
> > It would be useful, albeit tedious, to add get/set test pairs for each 
> > property.
> I think we have the basic functionality of the getter and setter covered. 
> Testing each getter/setter for each property would mostly help to verify the 
> mapping in the getters/setters - initially I've ruled out that possibility 
> with the macros (once the macro is correct - but that's much easier to 
> check). I would prefer to go back to the macro version than adding  a test 
> here that goes over all properties.
I think the macro in implementation harms readability and the tests are pretty 
simple to add (maybe even using a macro). 

The current setup of a switch and no low-level tests leaves things open to 
breakage in future.


Repository:
  rC Clang

https://reviews.llvm.org/D39903



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

[PATCH] D41989: [CodeComplete] Add an option to omit results from the preamble.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is basically all plumbing, so I've marked the two lines that actually do 
something :-)




Comment at: lib/Sema/SemaLookup.cpp:3517
 // Ensure all external identifiers are in the identifier table.
-if (IdentifierInfoLookup *External = Idents.getExternalIdentifierLookup()) 
{
-  std::unique_ptr Iter(External->getIdentifiers());
-  for (StringRef Name = Iter->Next(); !Name.empty(); Name = Iter->Next())
-Idents.get(Name);
-}
+if (Load)
+  if (IdentifierInfoLookup *External = 
Idents.getExternalIdentifierLookup()) {

here's a substantive change



Comment at: lib/Sema/SemaLookup.cpp:3546
+  for (DeclContextLookupResult R :
+   Load ? Ctx->lookups() : Ctx->noload_lookups()) {
 for (auto *D : R) {

here's a substantive change


Repository:
  rC Clang

https://reviews.llvm.org/D41989



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

[PATCH] D41990: [Frontend] Allow to use PrecompiledPreamble without calling CanReuse

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: bkramer, sammccall.

The new method 'OverridePreamble' allows to override the preamble of
any source file without checking if preamble bounds or dependencies
were changed.

This is used for completion in clangd.


Repository:
  rC Clang

https://reviews.llvm.org/D41990

Files:
  include/clang/Frontend/PrecompiledPreamble.h
  lib/Frontend/PrecompiledPreamble.cpp

Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -485,20 +485,15 @@
 void PrecompiledPreamble::AddImplicitPreamble(
 CompilerInvocation &CI, IntrusiveRefCntPtr &VFS,
 llvm::MemoryBuffer *MainFileBuffer) const {
-  assert(VFS && "VFS must not be null");
-
-  auto &PreprocessorOpts = CI.getPreprocessorOpts();
-
-  // Remap main file to point to MainFileBuffer.
-  auto MainFilePath = CI.getFrontendOpts().Inputs[0].getFile();
-  PreprocessorOpts.addRemappedFile(MainFilePath, MainFileBuffer);
-
-  // Configure ImpicitPCHInclude.
-  PreprocessorOpts.PrecompiledPreambleBytes.first = PreambleBytes.size();
-  PreprocessorOpts.PrecompiledPreambleBytes.second = PreambleEndsAtStartOfLine;
-  PreprocessorOpts.DisablePCHValidation = true;
+  PreambleBounds Bounds(PreambleBytes.size(), PreambleEndsAtStartOfLine);
+  configurePreamble(Bounds, CI, VFS, MainFileBuffer);
+}
 
-  setupPreambleStorage(Storage, PreprocessorOpts, VFS);
+void PrecompiledPreamble::OverridePreamble(
+CompilerInvocation &CI, IntrusiveRefCntPtr &VFS,
+llvm::MemoryBuffer *MainFileBuffer) const {
+  auto Bounds = ComputePreambleBounds(*CI.getLangOpts(), MainFileBuffer, 0);
+  configurePreamble(Bounds, CI, VFS, MainFileBuffer);
 }
 
 PrecompiledPreamble::PrecompiledPreamble(
@@ -681,6 +676,27 @@
   return Result;
 }
 
+void PrecompiledPreamble::configurePreamble(
+PreambleBounds Bounds, CompilerInvocation &CI,
+IntrusiveRefCntPtr &VFS,
+llvm::MemoryBuffer *MainFileBuffer) const {
+  assert(VFS && "VFS must not be null");
+
+  auto &PreprocessorOpts = CI.getPreprocessorOpts();
+
+  // Remap main file to point to MainFileBuffer.
+  auto MainFilePath = CI.getFrontendOpts().Inputs[0].getFile();
+  PreprocessorOpts.addRemappedFile(MainFilePath, MainFileBuffer);
+
+  // Configure ImpicitPCHInclude.
+  PreprocessorOpts.PrecompiledPreambleBytes.first = Bounds.Size;
+  PreprocessorOpts.PrecompiledPreambleBytes.second =
+  Bounds.PreambleEndsAtStartOfLine;
+  PreprocessorOpts.DisablePCHValidation = true;
+
+  setupPreambleStorage(Storage, PreprocessorOpts, VFS);
+}
+
 void PrecompiledPreamble::setupPreambleStorage(
 const PCHStorage &Storage, PreprocessorOptions &PreprocessorOpts,
 IntrusiveRefCntPtr &VFS) {
Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -104,14 +104,26 @@
   /// Changes options inside \p CI to use PCH from this preamble. Also remaps
   /// main file to \p MainFileBuffer and updates \p VFS to ensure the preamble
   /// is accessible.
-  /// For in-memory preambles, PrecompiledPreamble instance continues to own
-  /// the MemoryBuffer with the Preamble after this method returns. The caller
-  /// is reponsible for making sure the PrecompiledPreamble instance outlives
-  /// the compiler run and the AST that will be using the PCH.
+  /// Callers of this function must ensure that that CanReuse() returns true for
+  /// the specified file before calling this function.
+  /// For in-memory preambles, PrecompiledPreamble instance continues to own the
+  /// MemoryBuffer with the Preamble after this method returns. The caller is
+  /// reponsible for making sure the PrecompiledPreamble instance outlives the
+  /// compiler run and the AST that will be using the PCH.
   void AddImplicitPreamble(CompilerInvocation &CI,
IntrusiveRefCntPtr &VFS,
llvm::MemoryBuffer *MainFileBuffer) const;
 
+  /// Configure \p CI to pretend \p MainFileBuffer has a preamble, identical to
+  /// the PCH stored by this instance. This function is similar to
+  /// AddImplicitPreamble, but does require checking that CanReuse() returns
+  /// true prior to calling this function. Using this function allows to avoid
+  /// the expensive CanReuse() call and rebuild of the preamble, but results of
+  /// parsing the file may be incorrect.
+  void OverridePreamble(CompilerInvocation &CI,
+IntrusiveRefCntPtr &VFS,
+llvm::MemoryBuffer *MainFileBuffer) const;
+
 private:
   PrecompiledPreamble(PCHStorage Storage, std::vector PreambleBytes,
   bool PreambleEndsAtStartOfLine,
@@ -222,6 +234,12 @@
 }
   };
 
+  /// Helper function to set up PCH for the preamble into \p CI and \p VFS to

[PATCH] D41991: [clangd] Always use preamble (even stale) for code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: bkramer, sammccall.
Herald added a subscriber: klimek.

This improves performance of code completion, because we avoid stating
the files from the preamble and never attempt to parse the files
without using the preamble if it's provided.

However, the change comes at a cost of sometimes providing incorrect
results when doing code completion after making actually considerable
changes to the files used in the preamble or the preamble itself.
Eventually the preamble will get rebuilt and code completion will
be providing the correct results.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41991

Files:
  clangd/CodeComplete.cpp
  clangd/Compiler.cpp
  clangd/Compiler.h


Index: clangd/Compiler.h
===
--- clangd/Compiler.h
+++ clangd/Compiler.h
@@ -29,12 +29,14 @@
 };
 
 /// Creates a CompilerInstance with the main file contens overridden.
-/// The preamble will be reused unless it is null.
-/// Note that the vfs::FileSystem inside returned instance may differ if
-/// additional file remappings occur in command-line arguments.
-/// On some errors, returns null. When non-null value is returned, it's 
expected
-/// to be consumed by the FrontendAction as it will have a pointer to the
-/// MainFile buffer that will only be deleted if BeginSourceFile is called.
+/// The preamble will be set up via PrecompiledPreamble::OverridePreamble
+/// without doing any extra checks, the callers are responsible for checking
+/// PrecompiledPreamble::CanReuse if they need to make sure only up-to-date
+/// preambles are used . Note that the vfs::FileSystem inside returned instance
+/// may differ if additional file remappings occur in command-line arguments. 
On
+/// some errors, returns null. When non-null value is returned, it's expected 
to
+/// be consumed by the FrontendAction as it will have a pointer to the MainFile
+/// buffer that will only be deleted if BeginSourceFile is called.
 std::unique_ptr prepareCompilerInstance(
 std::unique_ptr, const PrecompiledPreamble *,
 std::unique_ptr MainFile,
Index: clangd/Compiler.cpp
===
--- clangd/Compiler.cpp
+++ clangd/Compiler.cpp
@@ -36,7 +36,7 @@
   // NOTE: we use Buffer.get() when adding remapped files, so we have to make
   // sure it will be released if no error is emitted.
   if (Preamble) {
-Preamble->AddImplicitPreamble(*CI, VFS, Buffer.get());
+Preamble->OverridePreamble(*CI, VFS, Buffer.get());
   } else {
 CI->getPreprocessorOpts().addRemappedFile(
 CI->getFrontendOpts().Inputs[0].getFile(), Buffer.get());
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -519,14 +519,11 @@
   std::unique_ptr ContentsBuffer =
   llvm::MemoryBuffer::getMemBufferCopy(Contents, FileName);
 
-  // Attempt to reuse the PCH from precompiled preamble, if it was built.
-  if (Preamble) {
-auto Bounds =
-ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
-if (!Preamble->CanReuse(*CI, ContentsBuffer.get(), Bounds, VFS.get()))
-  Preamble = nullptr;
-  }
-
+  // Note that we delibirately don't check if preamble is up-to-date. This
+  // operation is very expensive and we feel the right trade-off here is to
+  // trade correctness in some cases (preamble has not been rebuilt after
+  // changes to a file) for performance. Eventually the preamble will be 
rebuilt
+  // and code completion will produce correct results.
   auto Clang = prepareCompilerInstance(
   std::move(CI), Preamble, std::move(ContentsBuffer), std::move(PCHs),
   std::move(VFS), DummyDiagsConsumer);


Index: clangd/Compiler.h
===
--- clangd/Compiler.h
+++ clangd/Compiler.h
@@ -29,12 +29,14 @@
 };
 
 /// Creates a CompilerInstance with the main file contens overridden.
-/// The preamble will be reused unless it is null.
-/// Note that the vfs::FileSystem inside returned instance may differ if
-/// additional file remappings occur in command-line arguments.
-/// On some errors, returns null. When non-null value is returned, it's expected
-/// to be consumed by the FrontendAction as it will have a pointer to the
-/// MainFile buffer that will only be deleted if BeginSourceFile is called.
+/// The preamble will be set up via PrecompiledPreamble::OverridePreamble
+/// without doing any extra checks, the callers are responsible for checking
+/// PrecompiledPreamble::CanReuse if they need to make sure only up-to-date
+/// preambles are used . Note that the vfs::FileSystem inside returned instance
+/// may differ if additional file remappings occur in command-line arguments. On
+/// some errors, returns null. When non-null value is returned, it's expected to
+/// be consumed by the Fr

[clang-tools-extra] r322370 - [clangd] Don't navigate to forward class declaration when go to definition.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Jan 12 06:21:10 2018
New Revision: 322370

URL: http://llvm.org/viewvc/llvm-project?rev=322370&view=rev
Log:
[clangd] Don't navigate to forward class declaration when go to definition.

Summary:
For some cases, GoToDefinition will navigate to the forward class
declaration, we should always navigate to the class definition.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, ilya-biryukov, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=322370&r1=322369&r2=322370&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Jan 12 06:21:10 2018
@@ -14,6 +14,20 @@ namespace clangd {
 using namespace llvm;
 namespace {
 
+// Get the definition from a given declaration `D`.
+// Return nullptr if no definition is found, or the declaration type of `D` is
+// not supported.
+const Decl* GetDefinition(const Decl* D) {
+  assert(D);
+  if (const auto *TD = dyn_cast(D))
+return TD->getDefinition();
+  else if (const auto *VD = dyn_cast(D))
+return VD->getDefinition();
+  else if (const auto *FD = dyn_cast(D))
+return FD->getDefinition();
+  return nullptr;
+}
+
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
   std::vector Decls;
@@ -50,8 +64,18 @@ public:
   ArrayRef Relations, FileID FID,
   unsigned Offset,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-if (isSearchedLocation(FID, Offset))
-  Decls.push_back(D);
+if (isSearchedLocation(FID, Offset)) {
+  // Find and add definition declarations (for GoToDefinition).
+  // We don't use parameter `D`, as Parameter `D` is the canonical
+  // declaration, which is the first declaration of a redeclarable
+  // declaration, and it could be a forward declaration.
+  if (const auto* Def = GetDefinition(D)) {
+Decls.push_back(Def);
+  } else {
+// Couldn't find a definition, fall back to use `D`.
+Decls.push_back(D);
+  }
+}
 return true;
   }
 

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=322370&r1=322369&r2=322370&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Fri Jan 12 06:21:10 
2018
@@ -203,6 +203,18 @@ TEST(GoToDefinition, All) {
 #define MACRO 2
 #undef macro
   )cpp",
+
+  R"cpp(// Forward class declaration
+class Foo;
+[[class Foo {}]];
+F^oo* foo();
+  )cpp",
+
+  R"cpp(// Function declaration
+void foo();
+void g() { f^oo(); }
+[[void foo() {}]]
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);


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

[PATCH] D41661: [clangd] Don't navigate to forward class declaration when go to definition.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL322370: [clangd] Don't navigate to forward class 
declaration when go to definition. (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D41661

Files:
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -203,6 +203,18 @@
 #define MACRO 2
 #undef macro
   )cpp",
+
+  R"cpp(// Forward class declaration
+class Foo;
+[[class Foo {}]];
+F^oo* foo();
+  )cpp",
+
+  R"cpp(// Function declaration
+void foo();
+void g() { f^oo(); }
+[[void foo() {}]]
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -14,6 +14,20 @@
 using namespace llvm;
 namespace {
 
+// Get the definition from a given declaration `D`.
+// Return nullptr if no definition is found, or the declaration type of `D` is
+// not supported.
+const Decl* GetDefinition(const Decl* D) {
+  assert(D);
+  if (const auto *TD = dyn_cast(D))
+return TD->getDefinition();
+  else if (const auto *VD = dyn_cast(D))
+return VD->getDefinition();
+  else if (const auto *FD = dyn_cast(D))
+return FD->getDefinition();
+  return nullptr;
+}
+
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
   std::vector Decls;
@@ -50,8 +64,18 @@
   ArrayRef Relations, FileID FID,
   unsigned Offset,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-if (isSearchedLocation(FID, Offset))
-  Decls.push_back(D);
+if (isSearchedLocation(FID, Offset)) {
+  // Find and add definition declarations (for GoToDefinition).
+  // We don't use parameter `D`, as Parameter `D` is the canonical
+  // declaration, which is the first declaration of a redeclarable
+  // declaration, and it could be a forward declaration.
+  if (const auto* Def = GetDefinition(D)) {
+Decls.push_back(Def);
+  } else {
+// Couldn't find a definition, fall back to use `D`.
+Decls.push_back(D);
+  }
+}
 return true;
   }
 


Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -203,6 +203,18 @@
 #define MACRO 2
 #undef macro
   )cpp",
+
+  R"cpp(// Forward class declaration
+class Foo;
+[[class Foo {}]];
+F^oo* foo();
+  )cpp",
+
+  R"cpp(// Function declaration
+void foo();
+void g() { f^oo(); }
+[[void foo() {}]]
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -14,6 +14,20 @@
 using namespace llvm;
 namespace {
 
+// Get the definition from a given declaration `D`.
+// Return nullptr if no definition is found, or the declaration type of `D` is
+// not supported.
+const Decl* GetDefinition(const Decl* D) {
+  assert(D);
+  if (const auto *TD = dyn_cast(D))
+return TD->getDefinition();
+  else if (const auto *VD = dyn_cast(D))
+return VD->getDefinition();
+  else if (const auto *FD = dyn_cast(D))
+return FD->getDefinition();
+  return nullptr;
+}
+
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
   std::vector Decls;
@@ -50,8 +64,18 @@
   ArrayRef Relations, FileID FID,
   unsigned Offset,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-if (isSearchedLocation(FID, Offset))
-  Decls.push_back(D);
+if (isSearchedLocation(FID, Offset)) {
+  // Find and add definition declarations (for GoToDefinition).
+  // We don't use parameter `D`, as Parameter `D` is the canonical
+  // declaration, which is the first declaration of a redeclarable
+  // declaration, and it could be a forward declaration.
+  if (const auto* Def = GetDefinition(D)) {
+Decls.push_back(Def);
+  } else {

[PATCH] D41992: [libcxx] Avoid spurious construction of valarray elements

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki created this revision.
miyuki added reviewers: EricWF, mclow.lists.

Currently libc++ implements some operations on valarray by using the
resize method. This method has a parameter with a default value.
Because of this, valarray may spuriously construct and destruct
objects of valarray's element type.

  

This patch fixes this issue and adds corresponding test cases.


https://reviews.llvm.org/D41992

Files:
  include/valarray
  
test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp
  
test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp
  test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp
  test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp

Index: test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp
===
--- test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp
+++ test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp
@@ -16,6 +16,15 @@
 #include 
 #include 
 
+struct S {
+S() : x(1) {}
+~S() { ++cnt_dtor; }
+int x;
+static size_t cnt_dtor;
+};
+
+size_t S::cnt_dtor = 0;
+
 int main()
 {
 {
@@ -36,4 +45,11 @@
 for (int i = 0; i < 100; ++i)
 assert(v[i].size() == 0);
 }
+{
+std::valarray v(100);
+assert(v.size() == 100);
+for (int i = 0; i < 100; ++i)
+assert(v[i].x == 1);
+}
+assert(S::cnt_dtor == 100);
 }
Index: test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp
===
--- test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp
+++ test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp
@@ -16,6 +16,13 @@
 #include 
 #include 
 
+struct S {
+S() { ctor_called = true; }
+static bool ctor_called;
+};
+
+bool S::ctor_called = false;
+
 int main()
 {
 {
@@ -34,4 +41,9 @@
 std::valarray > v;
 assert(v.size() == 0);
 }
+{
+std::valarray v;
+assert(v.size() == 0);
+assert(!S::ctor_called);
+}
 }
Index: test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp
===
--- test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp
+++ test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp
@@ -19,6 +19,21 @@
 #include 
 #include 
 
+struct S
+{
+S() : x_(0) { default_ctor_called = true; }
+S(int x) : x_(x) {}
+int x_;
+static bool default_ctor_called;
+};
+
+bool S::default_ctor_called = false;
+
+bool operator==(const S& lhs, const S& rhs)
+{
+return lhs.x_ == rhs.x_;
+}
+
 int main()
 {
 {
@@ -55,4 +70,15 @@
 assert(v2[i][j] == a[i][j]);
 }
 }
+{
+typedef S T;
+T a[] = {T(1), T(2), T(3), T(4), T(5)};
+const unsigned N = sizeof(a)/sizeof(a[0]);
+std::valarray v2;
+v2 = {T(1), T(2), T(3), T(4), T(5)};
+assert(v2.size() == N);
+for (std::size_t i = 0; i < v2.size(); ++i)
+assert(v2[i] == a[i]);
+assert(!S::default_ctor_called);
+}
 }
Index: test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp
===
--- test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp
+++ test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp
@@ -17,6 +17,21 @@
 #include 
 #include 
 
+struct S
+{
+S() : x_(0) { default_ctor_called = true; }
+S(int x) : x_(x) {}
+int x_;
+static bool default_ctor_called;
+};
+
+bool S::default_ctor_called = false;
+
+bool operator==(const S& lhs, const S& rhs)
+{
+return lhs.x_ == rhs.x_;
+}
+
 int main()
 {
 {
@@ -56,4 +71,16 @@
 assert(v2[i][j] == v[i][j]);
 }
 }
+{
+typedef S T;
+T a[] = {T(1), T(2), T(3), T(4), T(5)};
+const unsigned N = sizeof(a)/sizeof(a[0]);
+std::valarray v(a, N);
+std::valarray v2;
+v2 = v;
+assert(v2.size() == v.size());
+for (std::size_t i = 0; i < v2.size(); ++i)
+assert(v2[i] == v[i]);
+assert(!S::default_ctor_called);
+}
 }
Index: include/valarray
===
--- include/valarray
+++ include/valarray
@@ -1053,6 +1053,9 @@
 friend
 const _Up*
 end(const valarray<_Up>& __v);
+
+void __clear();
+valarray& __assign_range(const value_type* __f, const value_type* __l);
 };
 
 _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::valarray(size_t))
@@ -2750,7 +2753,24 @@
 : __begin_(0),
   __end_(0)
 {

[PATCH] D41989: [CodeComplete] Add an option to omit results from the preamble.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM modulo the comment about variable name.




Comment at: lib/Sema/SemaLookup.cpp:3502
+   bool IncludeDependentBases,
+   bool Load) {
   if (!Ctx)

Maybe rename the variable to `LoadExternal`?
The intention of `Load` is not immediately clear. Also a bit longer name seems 
to be more in-line with the other code in this file.


Repository:
  rC Clang

https://reviews.llvm.org/D41989



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

[PATCH] D41937: [WebAssembly] supports -stdlib=libc++ switch

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

lgtm


https://reviews.llvm.org/D41937



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

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM, sorry for the delay.


https://reviews.llvm.org/D20124



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

[PATCH] D41990: [Frontend] Allow to use PrecompiledPreamble without calling CanReuse

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: include/clang/Frontend/PrecompiledPreamble.h:107
   /// is accessible.
-  /// For in-memory preambles, PrecompiledPreamble instance continues to own
-  /// the MemoryBuffer with the Preamble after this method returns. The caller
-  /// is reponsible for making sure the PrecompiledPreamble instance outlives
-  /// the compiler run and the AST that will be using the PCH.
+  /// Callers of this function must ensure that that CanReuse() returns true 
for
+  /// the specified file before calling this function.

Simpler just as "/// Requires that CanReuse() is true."



Comment at: include/clang/Frontend/PrecompiledPreamble.h:119
+  /// the PCH stored by this instance. This function is similar to
+  /// AddImplicitPreamble, but does require checking that CanReuse() returns
+  /// true prior to calling this function. Using this function allows to avoid

i think you mean "does not".

I find some of the wording here confusing - "does not require checking" seems 
like too weak a claim. It's also a bit long, and hard to quickly see at a 
glance which function you might want to call.

I'd suggest:
  /// Configure \CI to use this preamble for \p MainFileBuffer.
  /// Like AddImplicitPreamble, but doesn't assume or check CanReuse()!
  /// If this preamble doesn't match the file, it may parse differently.




Comment at: lib/Frontend/PrecompiledPreamble.cpp:683
+llvm::MemoryBuffer *MainFileBuffer) const {
+  assert(VFS && "VFS must not be null");
+

nit: this is just assert(VFS)


Repository:
  rC Clang

https://reviews.llvm.org/D41990



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

[PATCH] D41989: [CodeComplete] Add an option to omit results from the preamble.

[Sam McCall via Phabricator via cfe-commits]

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

Load -> LoadExternal


Repository:
  rC Clang

https://reviews.llvm.org/D41989

Files:
  include/clang-c/Index.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/CodeCompleteOptions.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaLookup.cpp
  test/Index/complete-pch-skip.cpp
  test/Index/complete-pch-skip.h
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -643,6 +643,7 @@
   ArrayRef unsaved_files,
   unsigned options) {
   bool IncludeBriefComments = options & CXCodeComplete_IncludeBriefComments;
+  bool SkipPreamble = options & CXCodeComplete_SkipPreamble;
 
 #ifdef UDP_CODE_COMPLETION_LOGGER
 #ifdef UDP_CODE_COMPLETION_LOGGER_PORT
@@ -689,6 +690,7 @@
   // Create a code-completion consumer to capture the results.
   CodeCompleteOptions Opts;
   Opts.IncludeBriefComments = IncludeBriefComments;
+  Opts.LoadExternal = !SkipPreamble;
   CaptureCompletionResults Capture(Opts, *Results, &TU);
 
   // Perform completion.
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -2326,6 +2326,8 @@
 completionOptions |= CXCodeComplete_IncludeCodePatterns;
   if (getenv("CINDEXTEST_COMPLETION_BRIEF_COMMENTS"))
 completionOptions |= CXCodeComplete_IncludeBriefComments;
+  if (getenv("CINDEXTEST_COMPLETION_SKIP_PREAMBLE"))
+completionOptions |= CXCodeComplete_SkipPreamble;
   
   if (timing_only)
 input += strlen("-code-completion-timing=");
Index: test/Index/complete-pch-skip.h
===
--- /dev/null
+++ test/Index/complete-pch-skip.h
@@ -0,0 +1,3 @@
+namespace ns {
+int foo;
+}
Index: test/Index/complete-pch-skip.cpp
===
--- /dev/null
+++ test/Index/complete-pch-skip.cpp
@@ -0,0 +1,18 @@
+namespace ns {
+int bar;
+}
+
+int main() { return ns:: }
+
+// RUN: echo "namespace ns { int foo; }" > %t.h
+// RUN: c-index-test -write-pch %t.h.pch -x c++-header %t.h
+//
+// RUN: c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=WITH-PCH %s
+// WITH-PCH: {TypedText bar}
+// WITH-PCH: {TypedText foo}
+
+// RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH %s
+// SKIP-PCH-NOT: foo
+// SKIP-PCH: {TypedText bar}
+// SKIP-PCH-NOT: foo
+
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3498,7 +3498,8 @@
bool InBaseClass,
VisibleDeclConsumer &Consumer,
VisibleDeclsRecord &Visited,
-   bool IncludeDependentBases = false) {
+   bool IncludeDependentBases,
+   bool LoadExternal) {
   if (!Ctx)
 return;
 
@@ -3513,11 +3514,12 @@
 auto &Idents = S.Context.Idents;
 
 // Ensure all external identifiers are in the identifier table.
-if (IdentifierInfoLookup *External = Idents.getExternalIdentifierLookup()) {
-  std::unique_ptr Iter(External->getIdentifiers());
-  for (StringRef Name = Iter->Next(); !Name.empty(); Name = Iter->Next())
-Idents.get(Name);
-}
+if (LoadExternal)
+  if (IdentifierInfoLookup *External = Idents.getExternalIdentifierLookup()) {
+std::unique_ptr Iter(External->getIdentifiers());
+for (StringRef Name = Iter->Next(); !Name.empty(); Name = Iter->Next())
+  Idents.get(Name);
+  }
 
 // Walk all lookup results in the TU for each identifier.
 for (const auto &Ident : Idents) {
@@ -3540,7 +3542,8 @@
 Result.getSema().ForceDeclarationOfImplicitMembers(Class);
 
   // Enumerate all of the results in this context.
-  for (DeclContextLookupResult R : Ctx->lookups()) {
+  for (DeclContextLookupResult R :
+   LoadExternal ? Ctx->lookups() : Ctx->noload_lookups()) {
 for (auto *D : R) {
   if (auto *ND = Result.getAcceptableDecl(D)) {
 Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
@@ -3557,7 +3560,7 @@
 continue;
   LookupVisibleDecls(I->getNominatedNamespace(), Result,
  QualifiedNameLookup, InBaseClass, Consumer, Visited,
- IncludeDependentBases);
+ IncludeDependentBases, LoadExternal);
 }
   }
 
@@ -3614,66 +3617,70 @@

r322371 - [CodeComplete] Add an option to omit results from the preamble.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Jan 12 06:51:47 2018
New Revision: 322371

URL: http://llvm.org/viewvc/llvm-project?rev=322371&view=rev
Log:
[CodeComplete] Add an option to omit results from the preamble.

Summary:
Enumerating the contents of a namespace or global scope will omit any
decls that aren't already loaded, instead of deserializing them from the
PCH.

This allows a fast hybrid code completion where symbols from headers are
provided by an external index. (Sema already exposes the information
needed to do a reasonabl job of filtering them).
Clangd plans to implement this hybrid.

This option is just a hint - callers still need to postfilter results if
they want to *avoid* completing decls outside the main file.

Reviewers: bkramer, ilya-biryukov

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/Index/complete-pch-skip.cpp
cfe/trunk/test/Index/complete-pch-skip.h
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=322371&r1=322370&r2=322371&view=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Fri Jan 12 06:51:47 2018
@@ -5161,7 +5161,14 @@ enum CXCodeComplete_Flags {
* \brief Whether to include brief documentation within the set of code
* completions returned.
*/
-  CXCodeComplete_IncludeBriefComments = 0x04
+  CXCodeComplete_IncludeBriefComments = 0x04,
+
+  /**
+   * \brief Whether to speed up completion by omitting some entities which are
+   * defined in the preamble. There's no guarantee any particular entity will
+   * be omitted. This may be useful if the headers are indexed externally.
+   */
+  CXCodeComplete_SkipPreamble = 0x08
 };
 
 /**

Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h?rev=322371&r1=322370&r2=322371&view=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Fri Jan 12 06:51:47 2018
@@ -934,6 +934,12 @@ public:
 return CodeCompleteOpts.IncludeBriefComments;
   }
 
+  /// \brief Hint whether to load data from the external AST in order to 
provide
+  /// full results. If false, declarations from the preamble may be omitted.
+  bool loadExternal() const {
+return CodeCompleteOpts.LoadExternal;
+  }
+
   /// \brief Determine whether the output of this consumer is binary.
   bool isOutputBinary() const { return OutputIsBinary; }
 

Modified: cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteOptions.h?rev=322371&r1=322370&r2=322371&view=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteOptions.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteOptions.h Fri Jan 12 06:51:47 2018
@@ -35,9 +35,14 @@ public:
   /// Show brief documentation comments in code completion results.
   unsigned IncludeBriefComments : 1;
 
+  /// Hint whether to load data from the external AST in order to provide
+  /// full results. If false, declarations from the preamble may be omitted.
+  unsigned LoadExternal : 1;
+
   CodeCompleteOptions()
   : IncludeMacros(0), IncludeCodePatterns(0), IncludeGlobals(1),
-IncludeNamespaceLevelDecls(1), IncludeBriefComments(0) {}
+IncludeNamespaceLevelDecls(1), IncludeBriefComments(0),
+LoadExternal(1) {}
 };
 
 } // namespace clang

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=322371&r1=322370&r2=322371&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan 12 06:51:47 2018
@@ -3199,11 +3199,13 @@ public:
 
   void LookupVisibleDecls(Scope *S, LookupNameKind Kind,
   VisibleDeclConsumer &Consumer,
-  bool IncludeGlobalScope = true);
+  bool IncludeGlobalScope = true,
+  bool LoadExternal = true);
   void LookupVisibleDecls(DeclContext *Ctx, LookupNameKind Kind,
   VisibleDeclConsumer &Consumer,
  

[PATCH] D41989: [CodeComplete] Add an option to omit results from the preamble.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC322371: [CodeComplete] Add an option to omit results from 
the preamble. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41989?vs=129618&id=129620#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41989

Files:
  include/clang-c/Index.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/CodeCompleteOptions.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaLookup.cpp
  test/Index/complete-pch-skip.cpp
  test/Index/complete-pch-skip.h
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndexCodeCompletion.cpp

Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3524,7 +3524,8 @@
   
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
- CodeCompleter->includeGlobals());
+ CodeCompleter->includeGlobals(),
+ CodeCompleter->loadExternal());
 
   AddOrdinaryNameResults(CompletionContext, S, *this, Results);
   Results.ExitScope();
@@ -3599,7 +3600,8 @@
   Results.setFilter(&ResultBuilder::IsImpossibleToSatisfy);
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(S, LookupNestedNameSpecifierName, Consumer,
- CodeCompleter->includeGlobals());
+ CodeCompleter->includeGlobals(),
+ CodeCompleter->loadExternal());
   Results.setFilter(nullptr);
 }
   }
@@ -3669,8 +3671,9 @@
   
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
- CodeCompleter->includeGlobals());
-  
+ CodeCompleter->includeGlobals(),
+ CodeCompleter->loadExternal());
+
   Results.EnterNewScope();
   AddOrdinaryNameResults(PCC_Expression, S, *this, Results);
   Results.ExitScope();
@@ -3939,7 +3942,8 @@
   CodeCompletionDeclConsumer Consumer(Results, SemaRef.CurContext);
   SemaRef.LookupVisibleDecls(RD, Sema::LookupMemberName, Consumer,
  SemaRef.CodeCompleter->includeGlobals(),
- /*IncludeDependentBases=*/true);
+ /*IncludeDependentBases=*/true,
+ SemaRef.CodeCompleter->loadExternal());
 
   if (SemaRef.getLangOpts().CPlusPlus) {
 if (!Results.empty()) {
@@ -4049,8 +4053,9 @@
 if (Class) {
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   Results.setFilter(&ResultBuilder::IsObjCIvar);
-  LookupVisibleDecls(Class, LookupMemberName, Consumer,
- CodeCompleter->includeGlobals());
+  LookupVisibleDecls(
+  Class, LookupMemberName, Consumer, CodeCompleter->includeGlobals(),
+  /*IncludeDependentBases=*/false, CodeCompleter->loadExternal());
 }
   }
   
@@ -4124,12 +4129,15 @@
   // First pass: look for tags.
   Results.setFilter(Filter);
   LookupVisibleDecls(S, LookupTagName, Consumer,
- CodeCompleter->includeGlobals());
+ CodeCompleter->includeGlobals(),
+ CodeCompleter->loadExternal());
 
   if (CodeCompleter->includeGlobals()) {
 // Second pass: look for nested name specifiers.
 Results.setFilter(&ResultBuilder::IsNestedNameSpecifier);
-LookupVisibleDecls(S, LookupNestedNameSpecifierName, Consumer);
+LookupVisibleDecls(S, LookupNestedNameSpecifierName, Consumer,
+   CodeCompleter->includeGlobals(),
+   CodeCompleter->loadExternal());
   }
   
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
@@ -4540,8 +4548,9 @@
   
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
- CodeCompleter->includeGlobals());
-  
+ CodeCompleter->includeGlobals(),
+ CodeCompleter->loadExternal());
+
   AddOrdinaryNameResults(PCC_Statement, S, *this, Results);
   
   // "else" block
@@ -4649,7 +4658,8 @@
 CodeCompletionDeclConsumer Consumer(Results, CurContext);
 LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
/*IncludeGlobalScope=*/true,
-   /*IncludeDependentBases=*/true);
+   /*IncludeDependentBases=*/true,
+   CodeCompleter->loadExternal());
   }
 
   auto CC = Results.getCompletionContext();
@@ -4677,7 +4687,8 @@
   // nested-name-specifier.
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
- CodeCompleter->includeGlobals());
+

[PATCH] D41989: [CodeComplete] Add an option to omit results from the preamble.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL322371: [CodeComplete] Add an option to omit results from 
the preamble. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D41989

Files:
  cfe/trunk/include/clang-c/Index.h
  cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
  cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Frontend/ASTUnit.cpp
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/lib/Sema/SemaLookup.cpp
  cfe/trunk/test/Index/complete-pch-skip.cpp
  cfe/trunk/test/Index/complete-pch-skip.h
  cfe/trunk/tools/c-index-test/c-index-test.c
  cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp

Index: cfe/trunk/include/clang-c/Index.h
===
--- cfe/trunk/include/clang-c/Index.h
+++ cfe/trunk/include/clang-c/Index.h
@@ -5161,7 +5161,14 @@
* \brief Whether to include brief documentation within the set of code
* completions returned.
*/
-  CXCodeComplete_IncludeBriefComments = 0x04
+  CXCodeComplete_IncludeBriefComments = 0x04,
+
+  /**
+   * \brief Whether to speed up completion by omitting some entities which are
+   * defined in the preamble. There's no guarantee any particular entity will
+   * be omitted. This may be useful if the headers are indexed externally.
+   */
+  CXCodeComplete_SkipPreamble = 0x08
 };
 
 /**
Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -3199,11 +3199,13 @@
 
   void LookupVisibleDecls(Scope *S, LookupNameKind Kind,
   VisibleDeclConsumer &Consumer,
-  bool IncludeGlobalScope = true);
+  bool IncludeGlobalScope = true,
+  bool LoadExternal = true);
   void LookupVisibleDecls(DeclContext *Ctx, LookupNameKind Kind,
   VisibleDeclConsumer &Consumer,
   bool IncludeGlobalScope = true,
-  bool IncludeDependentBases = false);
+  bool IncludeDependentBases = false,
+  bool LoadExternal = true);
 
   enum CorrectTypoKind {
 CTK_NonError, // CorrectTypo used in a non error recovery situation.
Index: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
===
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
@@ -934,6 +934,12 @@
 return CodeCompleteOpts.IncludeBriefComments;
   }
 
+  /// \brief Hint whether to load data from the external AST in order to provide
+  /// full results. If false, declarations from the preamble may be omitted.
+  bool loadExternal() const {
+return CodeCompleteOpts.LoadExternal;
+  }
+
   /// \brief Determine whether the output of this consumer is binary.
   bool isOutputBinary() const { return OutputIsBinary; }
 
Index: cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
===
--- cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
+++ cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
@@ -35,9 +35,14 @@
   /// Show brief documentation comments in code completion results.
   unsigned IncludeBriefComments : 1;
 
+  /// Hint whether to load data from the external AST in order to provide
+  /// full results. If false, declarations from the preamble may be omitted.
+  unsigned LoadExternal : 1;
+
   CodeCompleteOptions()
   : IncludeMacros(0), IncludeCodePatterns(0), IncludeGlobals(1),
-IncludeNamespaceLevelDecls(1), IncludeBriefComments(0) {}
+IncludeNamespaceLevelDecls(1), IncludeBriefComments(0),
+LoadExternal(1) {}
 };
 
 } // namespace clang
Index: cfe/trunk/test/Index/complete-pch-skip.h
===
--- cfe/trunk/test/Index/complete-pch-skip.h
+++ cfe/trunk/test/Index/complete-pch-skip.h
@@ -0,0 +1,3 @@
+namespace ns {
+int foo;
+}
Index: cfe/trunk/test/Index/complete-pch-skip.cpp
===
--- cfe/trunk/test/Index/complete-pch-skip.cpp
+++ cfe/trunk/test/Index/complete-pch-skip.cpp
@@ -0,0 +1,18 @@
+namespace ns {
+int bar;
+}
+
+int main() { return ns:: }
+
+// RUN: echo "namespace ns { int foo; }" > %t.h
+// RUN: c-index-test -write-pch %t.h.pch -x c++-header %t.h
+//
+// RUN: c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=WITH-PCH %s
+// WITH-PCH: {TypedText bar}
+// WITH-PCH: {TypedText foo}
+
+// RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH %s
+// SKIP-PCH-NOT: foo
+// S

[PATCH] D40780: [clangd] Incorporate fuzzy-match into result rankings.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added a comment.

Tested in this file:

  #include 
  #include 
  #include 
  int main() { std::^ }

Before this change:

  u -> __has_argument_type, __has_first_argument_type...
  un -> __has_argument_type, __has_first_argument_type...
  u_p -> __has_argument_type, __has_first_argument_type...
  um -> uintmax_t, unordered_map, unordered_multimap...

After this change

  u -> u16streampos, u16string, u32streampos...
  un -> unary_function, unary_negate, uncaught_exception...
  u_p -> unique_ptr, undeclare_no_pointers, __has_argument_type...
  um -> unordered_map, unordered_multimap, __has_argument_type...




Comment at: clangd/ClangdUnit.cpp:377
+  : Result(&Result), SymbolScore(score(Result)), FilterScore(FilterScore),
+Score(FilterScore * SymbolScore) {}
 

ioeric wrote:
> It might worth mentioning how well `FilterScore * SymbolScore` performs. I 
> think it could be affected by the distribution of the filtering score and 
> symbol scores. We might want to do some tweaks on the numbers depending on 
> the distributions...
Described this sensitivity.



Comment at: clangd/ClangdUnit.cpp:380
   CodeCompletionResult *Result;
-  float Score; // 0 to 1, higher is better.
+  float SymbolScore; // higher is better
+  float FilterScore; // 0 to 1, higher is better.

ioeric wrote:
> Any reason not to use `CompletionItemScores` here? Maybe copy over some 
> comments?
Done. My feeling was that CompletionItemScores was an API and we shouldn't 
couple our internal layout to it. But it's convenient now and may remain so, we 
can change it later.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40780



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

[PATCH] D40780: [clangd] Incorporate fuzzy-match into result rankings.

[Sam McCall via Phabricator via cfe-commits]

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

Rebase, and address comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40780

Files:
  clangd/CodeComplete.cpp
  clangd/FuzzyMatch.h
  clangd/Protocol.h
  test/clangd/completion-fuzzy.test

Index: test/clangd/completion-fuzzy.test
===
--- /dev/null
+++ test/clangd/completion-fuzzy.test
@@ -0,0 +1,38 @@
+# RUN: clangd -pretty -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 224
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"struct fake { int BigBang, Babble, Ball; };\nint main() {\n  fake f;\n  f.bb\n}\n"}}}
+
+# Complete right after the dot (no identifier)
+Content-Length: 148
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":3,"character":5}}}
+#  CHECK:  "id": 1
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+#  CHECK:  "label": "Babble"
+#  CHECK:  "label": "Ball"
+#  CHECK:  "label": "BigBang"
+
+# Complete after .bb.
+# BigBang is a better match than Babble, Ball doesn't match at all.
+Content-Length: 148
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":3,"character":6}}}
+#  CHECK:  "id": 2
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+#  CHECK-NOT:  "label": "Ball"
+#  CHECK:  "label": "BigBang"
+#  CHECK-NOT:  "label": "Ball"
+#  CHECK:  "label": "Babble"
+#  CHECK-NOT:  "label": "Ball"
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -446,6 +446,17 @@
   Snippet = 2,
 };
 
+/// Provides details for how a completion item was scored.
+/// This can be used for client-side filtering of completion items as the
+/// user keeps typing.
+/// This is a clangd extension.
+struct CompletionItemScores {
+  float finalScore;  /// The score that items are ranked by.
+ /// This is filterScore * symbolScore.
+  float filterScore; /// How the partial identifier matched filterText. [0-1]
+  float symbolScore; /// How the symbol fits, ignoring the partial identifier.
+};
+
 struct CompletionItem {
   /// The label of this completion item. By default also the text that is
   /// inserted when selecting this completion.
@@ -466,6 +477,9 @@
   /// When `falsy` the label is used.
   std::string sortText;
 
+  /// Details about the quality of this completion item. (clangd extension)
+  llvm::Optional scoreInfo;
+
   /// A string that should be used when filtering a set of completion items.
   /// When `falsy` the label is used.
   std::string filterText;
Index: clangd/FuzzyMatch.h
===
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -35,6 +35,8 @@
   // Characters beyond MaxWord are ignored.
   llvm::Optional match(llvm::StringRef Word);
 
+  bool empty() { return PatN == 0; }
+
   // Dump internal state from the last match() to the stream, for debugging.
   // Returns the pattern with [] around matched characters, e.g.
   //   [u_p] + "unique_ptr" --> "[u]nique[_p]tr"
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -17,6 +17,7 @@
 #include "CodeComplete.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
+#include "FuzzyMatch.h"
 #include "Logger.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -186,17 +187,25 @@
 
 /// A scored code completion result.
 /// It may be promoted to a CompletionItem if it's among the top-ranked results.
+///
+/// We score candidates by multiplying the symbolScore ("quality" of the result)
+/// with the filterScore (how well it matched the query).
+/// This is sensitive to the distribution of both component scores!
 struct CompletionCandidate {
-  CompletionCandidate(CodeCompletionResult &Result)
-  : Result(&Result), Score(score(Result)) {}
+  CompletionCandidate(CodeCompletionResult &Result, float FilterScore)
+  : Result(&Result) {
+Scores.symbolScore = score(Result);  // Higher is better.
+Scores.filterScore = FilterScore;// 0-1, higher is better.
+Scores.finalScore = Scores.symbolScore * Scores.filterScore;
+  }
 
   CodeCompletionResult *Result;
-  float Score; // 0 to 1, higher is better.
+  Completio

[PATCH] D41897: Fixing a crash in Sema.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: SemaCXX/base-class-ambiguity-check.cpp:1
+// RUN: %clang_cc1 -fsyntax-only %s
+

jkorous-apple wrote:
> aaron.ballman wrote:
> > jkorous-apple wrote:
> > > aaron.ballman wrote:
> > > > This run line isn't testing anything. Since you're trying to ensure 
> > > > this doesn't crash, I would put `-verify` on the RUN line and `// 
> > > > expected-no-diagnostics` on the line below.
> > > I know what you mean. I was worried about that as well so I asked other 
> > > guys. Apparently this is enough. If you run the test on master this is 
> > > what you get:
> > > 
> > > 
> > > ```
> > > > bin/llvm-lit 
> > > > /Users/jankorous/src/oss/llvm/llvm/tools/clang/test/SemaCXX/base-class-ambiguity-check.cpp
> > > llvm-lit: 
> > > /Users/jankorous/src/oss/llvm/llvm/utils/lit/lit/llvm/config.py:334: 
> > > note: using clang: /Users/jankorous/src/oss/llvm/build/bin/clang
> > > llvm-lit: /Users/jankorous/src/oss/llvm/llvm/utils/lit/lit/util.py:379: 
> > > note: using SDKROOT: 
> > > '/Applications/Edge9E77_AllSDKs_fromNFA/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk'
> > > -- Testing: 1 tests, 1 threads --
> > > FAIL: Clang :: SemaCXX/base-class-ambiguity-check.cpp (1 of 1)
> > > Testing Time: 0.31s
> > > 
> > > Failing Tests (1):
> > > Clang :: SemaCXX/base-class-ambiguity-check.cpp
> > > 
> > >   Unexpected Failures: 1
> > > 
> > > ```
> > This comment hasn't been applied yet.
> Sorry, what do you mean?
Weird, it seems Phab didn't have your latest comment when I posted mine -- 
sorry for the confusion!

"This is enough" != "This is the way it should be done" -- by adding `-verify` 
and `// expected-no-diagnostics`, you wind up testing that this code doesn't 
produce any diagnostics in addition to not crashing, which is an improvement 
over just testing there's no crash, as this code is perfectly reasonable and 
shouldn't produce a diagnostic.



Comment at: SemaCXX/base-class-ambiguity-check.cpp:6
+
+  // Up to commit 680b3a8619 (2018-01-10) this produced a crash in Sema.
+  struct Derived : Base, T {};

The commit hash doesn't do much here (we're still on svn, but the revision 
information isn't all that helpful), so I'd replace it with "// Test that this 
code no longer causes a crash in Sema. See PR/rdar://" (assuming you 
have a report somewhere; if not, just leave off the last sentence).


https://reviews.llvm.org/D41897



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

[PATCH] D41897: Fixing a crash in Sema.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added inline comments.



Comment at: SemaCXX/base-class-ambiguity-check.cpp:1
+// RUN: %clang_cc1 -fsyntax-only %s
+

aaron.ballman wrote:
> jkorous-apple wrote:
> > aaron.ballman wrote:
> > > jkorous-apple wrote:
> > > > aaron.ballman wrote:
> > > > > This run line isn't testing anything. Since you're trying to ensure 
> > > > > this doesn't crash, I would put `-verify` on the RUN line and `// 
> > > > > expected-no-diagnostics` on the line below.
> > > > I know what you mean. I was worried about that as well so I asked other 
> > > > guys. Apparently this is enough. If you run the test on master this is 
> > > > what you get:
> > > > 
> > > > 
> > > > ```
> > > > > bin/llvm-lit 
> > > > > /Users/jankorous/src/oss/llvm/llvm/tools/clang/test/SemaCXX/base-class-ambiguity-check.cpp
> > > > llvm-lit: 
> > > > /Users/jankorous/src/oss/llvm/llvm/utils/lit/lit/llvm/config.py:334: 
> > > > note: using clang: /Users/jankorous/src/oss/llvm/build/bin/clang
> > > > llvm-lit: /Users/jankorous/src/oss/llvm/llvm/utils/lit/lit/util.py:379: 
> > > > note: using SDKROOT: 
> > > > '/Applications/Edge9E77_AllSDKs_fromNFA/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk'
> > > > -- Testing: 1 tests, 1 threads --
> > > > FAIL: Clang :: SemaCXX/base-class-ambiguity-check.cpp (1 of 1)
> > > > Testing Time: 0.31s
> > > > 
> > > > Failing Tests (1):
> > > > Clang :: SemaCXX/base-class-ambiguity-check.cpp
> > > > 
> > > >   Unexpected Failures: 1
> > > > 
> > > > ```
> > > This comment hasn't been applied yet.
> > Sorry, what do you mean?
> Weird, it seems Phab didn't have your latest comment when I posted mine -- 
> sorry for the confusion!
> 
> "This is enough" != "This is the way it should be done" -- by adding 
> `-verify` and `// expected-no-diagnostics`, you wind up testing that this 
> code doesn't produce any diagnostics in addition to not crashing, which is an 
> improvement over just testing there's no crash, as this code is perfectly 
> reasonable and shouldn't produce a diagnostic.
No problem, I thought that we are probably not on the same page.

Fair enough. I can do that.



Comment at: SemaCXX/base-class-ambiguity-check.cpp:6
+
+  // Up to commit 680b3a8619 (2018-01-10) this produced a crash in Sema.
+  struct Derived : Base, T {};

aaron.ballman wrote:
> The commit hash doesn't do much here (we're still on svn, but the revision 
> information isn't all that helpful), so I'd replace it with "// Test that 
> this code no longer causes a crash in Sema. See PR/rdar://" (assuming 
> you have a report somewhere; if not, just leave off the last sentence).
Oh, right. Thanks.


https://reviews.llvm.org/D41897



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

[PATCH] D40780: [clangd] Incorporate fuzzy-match into result rankings.

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM.




Comment at: test/clangd/completion-fuzzy.test:1
+# RUN: clangd -pretty -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.

nit: any reason not using unittest `CodeCompleteTests` here? 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40780



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

[PATCH] D41897: Fixing a crash in Sema.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple updated this revision to Diff 129631.
jkorous-apple added a comment.

Fixed test based on Aaron's comments.


https://reviews.llvm.org/D41897

Files:
  Sema/SemaDeclCXX.cpp
  SemaCXX/base-class-ambiguity-check.cpp


Index: SemaCXX/base-class-ambiguity-check.cpp
===
--- /dev/null
+++ SemaCXX/base-class-ambiguity-check.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+template  class Foo {
+  struct Base : T {};
+
+  // Test that this code no longer causes a crash in Sema. rdar://23291875
+  struct Derived : Base, T {};
+};
Index: Sema/SemaDeclCXX.cpp
===
--- Sema/SemaDeclCXX.cpp
+++ Sema/SemaDeclCXX.cpp
@@ -2417,9 +2417,16 @@
   // Attach the remaining base class specifiers to the derived class.
   Class->setBases(Bases.data(), NumGoodBases);
 
+  // Check that the only base classes that are duplicate are virtual.
   for (unsigned idx = 0; idx < NumGoodBases; ++idx) {
 // Check whether this direct base is inaccessible due to ambiguity.
 QualType BaseType = Bases[idx]->getType();
+
+// Skip all dependent types in templates being used as base specifiers.
+// Checks below assume that the base specifier is a CXXRecord.
+if (BaseType->isDependentType())
+  continue;
+
 CanQualType CanonicalBase = Context.getCanonicalType(BaseType)
   .getUnqualifiedType();
 


Index: SemaCXX/base-class-ambiguity-check.cpp
===
--- /dev/null
+++ SemaCXX/base-class-ambiguity-check.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+template  class Foo {
+  struct Base : T {};
+
+  // Test that this code no longer causes a crash in Sema. rdar://23291875
+  struct Derived : Base, T {};
+};
Index: Sema/SemaDeclCXX.cpp
===
--- Sema/SemaDeclCXX.cpp
+++ Sema/SemaDeclCXX.cpp
@@ -2417,9 +2417,16 @@
   // Attach the remaining base class specifiers to the derived class.
   Class->setBases(Bases.data(), NumGoodBases);
 
+  // Check that the only base classes that are duplicate are virtual.
   for (unsigned idx = 0; idx < NumGoodBases; ++idx) {
 // Check whether this direct base is inaccessible due to ambiguity.
 QualType BaseType = Bases[idx]->getType();
+
+// Skip all dependent types in templates being used as base specifiers.
+// Checks below assume that the base specifier is a CXXRecord.
+if (BaseType->isDependentType())
+  continue;
+
 CanQualType CanonicalBase = Context.getCanonicalType(BaseType)
   .getUnqualifiedType();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41897: Fixing a crash in Sema.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you!


https://reviews.llvm.org/D41897



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

[PATCH] D41937: [WebAssembly] supports -stdlib=libc++ switch

[Patrick Cheng via Phabricator via cfe-commits]

patcheng added a comment.

Thanks @sbc100.

BTW I don't have commit access. When it's ready to be committed, I would need 
help with that.

Thanks


https://reviews.llvm.org/D41937



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

[PATCH] D41897: Fixing a crash in Sema.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added a comment.

Thank you!


https://reviews.llvm.org/D41897



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

[PATCH] D41897: Fixing a crash in Sema.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D41897#974618, @jkorous-apple wrote:

> Thank you!


Do you have commit privileges, or would you like me to commit this on your 
behalf?


https://reviews.llvm.org/D41897



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

[PATCH] D41897: Fixing a crash in Sema.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added a comment.

I do. Will commit this in a minute.


https://reviews.llvm.org/D41897



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

[PATCH] D40780: [clangd] Incorporate fuzzy-match into result rankings.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 129633.
sammccall marked an inline comment as done.
sammccall added a comment.

Unit test instead of lit test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40780

Files:
  clangd/CodeComplete.cpp
  clangd/FuzzyMatch.h
  clangd/Protocol.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -380,6 +380,31 @@
   EXPECT_THAT(Results.items, Has("namespace", CompletionItemKind::Snippet));
 }
 
+TEST(CompletionTest, NoDuplicates) {
+  auto Items = completions(R"cpp(
+struct Adapter {
+  void method();
+};
+
+void Adapter::method() {
+  Adapter^
+}
+  )cpp")
+   .items;
+
+  // Make sure there are no duplicate entries of 'Adapter'.
+  EXPECT_THAT(Items, ElementsAre(Named("Adapter"), Named("~Adapter")));
+}
+
+TEST(CompletionTest, FuzzyRanking) {
+  auto Items = completions(R"cpp(
+  struct fake { int BigBang, Babble, Ball; };
+  int main() { fake().bb^ }")cpp").items;
+  // BigBang is a better match than Babble. Ball doesn't match at all.
+  EXPECT_THAT(Items, ElementsAre(Named("BigBang"), Named("Babble")));
+}
+
+
 SignatureHelp signatures(StringRef Text) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
@@ -599,22 +624,6 @@
 Doc("Doooc"), Detail("void";
 }
 
-TEST(CompletionTest, NoDuplicates) {
-  auto Items = completions(R"cpp(
-struct Adapter {
-  void method();
-};
-
-void Adapter::method() {
-  Adapter^
-}
-  )cpp")
-   .items;
-
-  // Make sure there are no duplicate entries of 'Adapter'.
-  EXPECT_THAT(Items, ElementsAre(Named("Adapter"), Named("~Adapter")));
-}
-
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -446,6 +446,17 @@
   Snippet = 2,
 };
 
+/// Provides details for how a completion item was scored.
+/// This can be used for client-side filtering of completion items as the
+/// user keeps typing.
+/// This is a clangd extension.
+struct CompletionItemScores {
+  float finalScore;  /// The score that items are ranked by.
+ /// This is filterScore * symbolScore.
+  float filterScore; /// How the partial identifier matched filterText. [0-1]
+  float symbolScore; /// How the symbol fits, ignoring the partial identifier.
+};
+
 struct CompletionItem {
   /// The label of this completion item. By default also the text that is
   /// inserted when selecting this completion.
@@ -466,6 +477,9 @@
   /// When `falsy` the label is used.
   std::string sortText;
 
+  /// Details about the quality of this completion item. (clangd extension)
+  llvm::Optional scoreInfo;
+
   /// A string that should be used when filtering a set of completion items.
   /// When `falsy` the label is used.
   std::string filterText;
Index: clangd/FuzzyMatch.h
===
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -35,6 +35,8 @@
   // Characters beyond MaxWord are ignored.
   llvm::Optional match(llvm::StringRef Word);
 
+  bool empty() { return PatN == 0; }
+
   // Dump internal state from the last match() to the stream, for debugging.
   // Returns the pattern with [] around matched characters, e.g.
   //   [u_p] + "unique_ptr" --> "[u]nique[_p]tr"
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -17,6 +17,7 @@
 #include "CodeComplete.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
+#include "FuzzyMatch.h"
 #include "Logger.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -186,17 +187,25 @@
 
 /// A scored code completion result.
 /// It may be promoted to a CompletionItem if it's among the top-ranked results.
+///
+/// We score candidates by multiplying the symbolScore ("quality" of the result)
+/// with the filterScore (how well it matched the query).
+/// This is sensitive to the distribution of both component scores!
 struct CompletionCandidate {
-  CompletionCandidate(CodeCompletionResult &Result)
-  : Result(&Result), Score(score(Result)) {}
+  CompletionCandidate(CodeCompletionResult &Result, float FilterScore)
+  : Result(&Result) {
+Scores.symbolScore = score(Result);  // Higher is better.
+Scores.filterScore = FilterScore;// 0-1, higher is better.
+Scores.finalScore = Scores.symbolScore * Scores.filterScore;
+  }
 
   CodeCompletionResult *Result;
-  float Score; // 0 to 1, higher is better.
+  CompletionItemScores Scores;
 
   // Comparison reflects rank: better candidates are smaller.
   bool operator<(const CompletionCandidate &C) const {
-if (Score != C.Score)
-   

[PATCH] D40780: [clangd] Incorporate fuzzy-match into result rankings.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: test/clangd/completion-fuzzy.test:1
+# RUN: clangd -pretty -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.

hokein wrote:
> nit: any reason not using unittest `CodeCompleteTests` here? 
Whoops, none at all! Done.
When I first wrote this, CodeCompletionTests didn't exist in a useful form.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40780



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

[clang-tools-extra] r322377 - [clangd] Incorporate fuzzy-match into result rankings.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Jan 12 08:16:09 2018
New Revision: 322377

URL: http://llvm.org/viewvc/llvm-project?rev=322377&view=rev
Log:
[clangd] Incorporate fuzzy-match into result rankings.

Summary: The scoring function is fuzzy-match-quality * existing quality score.

Reviewers: ioeric

Subscribers: klimek, cfe-commits, ilya-biryukov

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/FuzzyMatch.h
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=322377&r1=322376&r2=322377&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Jan 12 08:16:09 2018
@@ -17,6 +17,7 @@
 #include "CodeComplete.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
+#include "FuzzyMatch.h"
 #include "Logger.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -186,17 +187,25 @@ getOptionalParameters(const CodeCompleti
 
 /// A scored code completion result.
 /// It may be promoted to a CompletionItem if it's among the top-ranked 
results.
+///
+/// We score candidates by multiplying the symbolScore ("quality" of the 
result)
+/// with the filterScore (how well it matched the query).
+/// This is sensitive to the distribution of both component scores!
 struct CompletionCandidate {
-  CompletionCandidate(CodeCompletionResult &Result)
-  : Result(&Result), Score(score(Result)) {}
+  CompletionCandidate(CodeCompletionResult &Result, float FilterScore)
+  : Result(&Result) {
+Scores.symbolScore = score(Result);  // Higher is better.
+Scores.filterScore = FilterScore;// 0-1, higher is better.
+Scores.finalScore = Scores.symbolScore * Scores.filterScore;
+  }
 
   CodeCompletionResult *Result;
-  float Score; // 0 to 1, higher is better.
+  CompletionItemScores Scores;
 
   // Comparison reflects rank: better candidates are smaller.
   bool operator<(const CompletionCandidate &C) const {
-if (Score != C.Score)
-  return Score > C.Score;
+if (Scores.finalScore != C.Scores.finalScore)
+  return Scores.finalScore > C.Scores.finalScore;
 return *Result < *C.Result;
   }
 
@@ -206,8 +215,8 @@ struct CompletionCandidate {
   std::string sortText() const {
 std::string S, NameStorage;
 llvm::raw_string_ostream OS(S);
-write_hex(OS, encodeFloat(-Score), llvm::HexPrintStyle::Lower,
-  /*Width=*/2 * sizeof(Score));
+write_hex(OS, encodeFloat(-Scores.finalScore), llvm::HexPrintStyle::Lower,
+  /*Width=*/2 * sizeof(Scores.finalScore));
 OS << Result->getOrderedName(NameStorage);
 return OS.str();
   }
@@ -288,6 +297,7 @@ public:
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override final {
+FuzzyMatcher Filter(S.getPreprocessor().getCodeCompletionFilter());
 if (auto SS = Context.getCXXScopeSpecifier())
   CompletedName.SSInfo = extraCompletionScope(S, **SS);
 
@@ -306,10 +316,10 @@ public:
   (Result.Availability == CXAvailability_NotAvailable ||
Result.Availability == CXAvailability_NotAccessible))
 continue;
-  if (!CompletedName.Filter.empty() &&
-  !fuzzyMatch(S, Context, CompletedName.Filter, Result))
+  auto FilterScore = fuzzyMatch(S, Context, Filter, Result);
+  if (!FilterScore)
 continue;
-  Candidates.emplace(Result);
+  Candidates.emplace(Result, *FilterScore);
   if (ClangdOpts.Limit && Candidates.size() > ClangdOpts.Limit) {
 Candidates.pop();
 Items.isIncomplete = true;
@@ -332,37 +342,24 @@ public:
   CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
 
 private:
-  bool fuzzyMatch(Sema &S, const CodeCompletionContext &CCCtx, StringRef 
Filter,
-  CodeCompletionResult Result) {
+  llvm::Optional fuzzyMatch(Sema &S, const CodeCompletionContext &CCCtx,
+   FuzzyMatcher &Filter,
+   CodeCompletionResult Result) {
 switch (Result.Kind) {
 case CodeCompletionResult::RK_Declaration:
   if (auto *ID = Result.Declaration->getIdentifier())
-return fuzzyMatch(Filter, ID->getName());
+return Filter.match(ID->getName());
   break;
 case CodeCompletionResult::RK_Keyword:
-  return fuzzyMatch(Filter, Result.Keyword);
+  return Filter.match(Result.Keyword);
 case CodeCompletionResult::RK_Macro:
-  return fuzzyMatch(Filter, Result.Macro->getName());
+  ret

[PATCH] D40780: [clangd] Incorporate fuzzy-match into result rankings.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL322377: [clangd] Incorporate fuzzy-match into result 
rankings. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D40780?vs=129633&id=129634#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40780

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/clangd/FuzzyMatch.h
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/clangd/FuzzyMatch.h
===
--- clang-tools-extra/trunk/clangd/FuzzyMatch.h
+++ clang-tools-extra/trunk/clangd/FuzzyMatch.h
@@ -35,6 +35,8 @@
   // Characters beyond MaxWord are ignored.
   llvm::Optional match(llvm::StringRef Word);
 
+  bool empty() { return PatN == 0; }
+
   // Dump internal state from the last match() to the stream, for debugging.
   // Returns the pattern with [] around matched characters, e.g.
   //   [u_p] + "unique_ptr" --> "[u]nique[_p]tr"
Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -446,6 +446,17 @@
   Snippet = 2,
 };
 
+/// Provides details for how a completion item was scored.
+/// This can be used for client-side filtering of completion items as the
+/// user keeps typing.
+/// This is a clangd extension.
+struct CompletionItemScores {
+  float finalScore;  /// The score that items are ranked by.
+ /// This is filterScore * symbolScore.
+  float filterScore; /// How the partial identifier matched filterText. [0-1]
+  float symbolScore; /// How the symbol fits, ignoring the partial identifier.
+};
+
 struct CompletionItem {
   /// The label of this completion item. By default also the text that is
   /// inserted when selecting this completion.
@@ -466,6 +477,9 @@
   /// When `falsy` the label is used.
   std::string sortText;
 
+  /// Details about the quality of this completion item. (clangd extension)
+  llvm::Optional scoreInfo;
+
   /// A string that should be used when filtering a set of completion items.
   /// When `falsy` the label is used.
   std::string filterText;
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -17,6 +17,7 @@
 #include "CodeComplete.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
+#include "FuzzyMatch.h"
 #include "Logger.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -186,17 +187,25 @@
 
 /// A scored code completion result.
 /// It may be promoted to a CompletionItem if it's among the top-ranked results.
+///
+/// We score candidates by multiplying the symbolScore ("quality" of the result)
+/// with the filterScore (how well it matched the query).
+/// This is sensitive to the distribution of both component scores!
 struct CompletionCandidate {
-  CompletionCandidate(CodeCompletionResult &Result)
-  : Result(&Result), Score(score(Result)) {}
+  CompletionCandidate(CodeCompletionResult &Result, float FilterScore)
+  : Result(&Result) {
+Scores.symbolScore = score(Result);  // Higher is better.
+Scores.filterScore = FilterScore;// 0-1, higher is better.
+Scores.finalScore = Scores.symbolScore * Scores.filterScore;
+  }
 
   CodeCompletionResult *Result;
-  float Score; // 0 to 1, higher is better.
+  CompletionItemScores Scores;
 
   // Comparison reflects rank: better candidates are smaller.
   bool operator<(const CompletionCandidate &C) const {
-if (Score != C.Score)
-  return Score > C.Score;
+if (Scores.finalScore != C.Scores.finalScore)
+  return Scores.finalScore > C.Scores.finalScore;
 return *Result < *C.Result;
   }
 
@@ -206,8 +215,8 @@
   std::string sortText() const {
 std::string S, NameStorage;
 llvm::raw_string_ostream OS(S);
-write_hex(OS, encodeFloat(-Score), llvm::HexPrintStyle::Lower,
-  /*Width=*/2 * sizeof(Score));
+write_hex(OS, encodeFloat(-Scores.finalScore), llvm::HexPrintStyle::Lower,
+  /*Width=*/2 * sizeof(Scores.finalScore));
 OS << Result->getOrderedName(NameStorage);
 return OS.str();
   }
@@ -288,6 +297,7 @@
   void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override final {
+FuzzyMatcher Filter(S.getPreprocessor().getCodeCompletionFilter());
 if (auto SS = Context.getCXXScopeSpecifier())
   CompletedName.SSInfo = extraCompletionScope(S, **SS);
 
@@ -306,10 +316,10 @@
   (Result.Availab

[PATCH] D39903: [libclang] Allow pretty printing declarations

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 129635.
nik added a comment.

What about this? :)


Repository:
  rC Clang

https://reviews.llvm.org/D39903

Files:
  include/clang-c/Index.h
  test/Index/print-display-names.cpp
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp
  tools/libclang/libclang.exports
  unittests/libclang/LibclangTest.cpp

Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -572,3 +572,33 @@
   EXPECT_EQ(0U, clang_getNumDiagnostics(ClangTU));
   DisplayDiagnostics();
 }
+
+class LibclangPrintingPolicyTest : public LibclangParseTest {
+public:
+  CXPrintingPolicy Policy = nullptr;
+
+  void SetUp() override {
+LibclangParseTest::SetUp();
+std::string File = "file.cpp";
+WriteFile(File, "int i;\n");
+ClangTU = clang_parseTranslationUnit(Index, File.c_str(), nullptr, 0,
+ nullptr, 0, TUFlags);
+CXCursor TUCursor = clang_getTranslationUnitCursor(ClangTU);
+Policy = clang_getCursorPrintingPolicy(TUCursor);
+  }
+  void TearDown() override {
+clang_PrintingPolicy_dispose(Policy);
+LibclangParseTest::TearDown();
+  }
+};
+
+TEST_F(LibclangPrintingPolicyTest, SetAndGetProperties) {
+  for (unsigned Value = 0; Value < 2; ++Value) {
+for (int I = 0; I < CXPrintingPolicy_LastProperty; ++I) {
+  auto Property = static_cast(I);
+
+  clang_PrintingPolicy_setProperty(Policy, Property, Value);
+  EXPECT_EQ(Value, clang_PrintingPolicy_getProperty(Policy, Property));
+}
+  }
+}
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -178,6 +178,8 @@
 clang_getCursorCompletionString
 clang_getCursorDefinition
 clang_getCursorDisplayName
+clang_getCursorPrintingPolicy
+clang_getCursorPrettyPrinted
 clang_getCursorExtent
 clang_getCursorExceptionSpecificationType
 clang_getCursorKind
@@ -359,3 +361,6 @@
 clang_EvalResult_getAsDouble
 clang_EvalResult_getAsStr
 clang_EvalResult_dispose
+clang_PrintingPolicy_getProperty
+clang_PrintingPolicy_setProperty
+clang_PrintingPolicy_dispose
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4706,6 +4706,195 @@
   return cxstring::createSet(Manglings);
 }
 
+CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) {
+  if (clang_Cursor_isNull(C))
+return 0;
+  return new PrintingPolicy(getCursorContext(C).getPrintingPolicy());
+}
+
+void clang_PrintingPolicy_dispose(CXPrintingPolicy Policy) {
+  if (Policy)
+delete static_cast(Policy);
+}
+
+unsigned
+clang_PrintingPolicy_getProperty(CXPrintingPolicy Policy,
+ enum CXPrintingPolicyProperty Property) {
+  if (!Policy)
+return 0;
+
+  PrintingPolicy *P = static_cast(Policy);
+  switch (Property) {
+  case CXPrintingPolicy_Indentation:
+return P->Indentation;
+  case CXPrintingPolicy_SuppressSpecifiers:
+return P->SuppressSpecifiers;
+  case CXPrintingPolicy_SuppressTagKeyword:
+return P->SuppressTagKeyword;
+  case CXPrintingPolicy_IncludeTagDefinition:
+return P->IncludeTagDefinition;
+  case CXPrintingPolicy_SuppressScope:
+return P->SuppressScope;
+  case CXPrintingPolicy_SuppressUnwrittenScope:
+return P->SuppressUnwrittenScope;
+  case CXPrintingPolicy_SuppressInitializers:
+return P->SuppressInitializers;
+  case CXPrintingPolicy_ConstantArraySizeAsWritten:
+return P->ConstantArraySizeAsWritten;
+  case CXPrintingPolicy_AnonymousTagLocations:
+return P->AnonymousTagLocations;
+  case CXPrintingPolicy_SuppressStrongLifetime:
+return P->SuppressStrongLifetime;
+  case CXPrintingPolicy_SuppressLifetimeQualifiers:
+return P->SuppressLifetimeQualifiers;
+  case CXPrintingPolicy_SuppressTemplateArgsInCXXConstructors:
+return P->SuppressTemplateArgsInCXXConstructors;
+  case CXPrintingPolicy_Bool:
+return P->Bool;
+  case CXPrintingPolicy_Restrict:
+return P->Restrict;
+  case CXPrintingPolicy_Alignof:
+return P->Alignof;
+  case CXPrintingPolicy_UnderscoreAlignof:
+return P->UnderscoreAlignof;
+  case CXPrintingPolicy_UseVoidForZeroParams:
+return P->UseVoidForZeroParams;
+  case CXPrintingPolicy_TerseOutput:
+return P->TerseOutput;
+  case CXPrintingPolicy_PolishForDeclaration:
+return P->PolishForDeclaration;
+  case CXPrintingPolicy_Half:
+return P->Half;
+  case CXPrintingPolicy_MSWChar:
+return P->MSWChar;
+  case CXPrintingPolicy_IncludeNewlines:
+return P->IncludeNewlines;
+  case CXPrintingPolicy_MSVCFormatting:
+return P->MSVCFormatting;
+  case CXPrintingPolicy_ConstantsAsWritten:
+return P->ConstantsAsWritten;
+  case CXPrintingPolicy_SuppressImplicitBase:
+return P->SuppressImplicitBase;
+  case

[PATCH] D41980: Add tests for llvm-bcanalyzer stream types

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: test/PCH/include-stream-type.cpp:7
+// RUN: cp %S/Inputs/pragma-once2.h %t-dir
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %t-dir/pragma-once2-pch.h
+// RUN: llvm-bcanalyzer -dump %t | FileCheck %s

You may need to pass an explicit -fmodule-format=raw for this to work on macOS. 
Did you test there?


Repository:
  rC Clang

https://reviews.llvm.org/D41980



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1937
+RetAttrs.addAttribute(llvm::Attribute::ZExt);
+}
 // FALL THROUGH

asb wrote:
> rjmccall wrote:
> > I feel like a better design would be to record whether to do a sext or a 
> > zext in the ABIArgInfo.  Add getSignExtend and getZeroExtend static 
> > functions to ABIArgInfo and make getExtend a convenience function that 
> > takes a QualType and uses its signedness.
> I could see how that might be cleaner, but that's a larger refactoring that's 
> going to touch a lot more code. Are you happy for this patch to stick with 
> this more incremental change (applying the same sign-extension logic to 
> return values as is used for arguments), and to leave your suggested 
> refactoring for a future patch?
I won't insist that you do it, but I don't think this refactor would be as bad 
as you think.  Doing these refactors incrementally when we realize that the 
existing infrastructure is failing us in some way is how we make sure they 
actually happen.  Individual contributors rarely have any incentive to ever do 
that "future patch".



Comment at: lib/CodeGen/TargetInfo.cpp:8858
+
+  uint64_t Size = getContext().getTypeSize(Ty);
+  uint64_t NeededAlign = getContext().getTypeAlign(Ty);

asb wrote:
> rjmccall wrote:
> > I would encourage you to use CharUnits and getTypeSizeInChars more 
> > consistently in this code; it would simplify some of the conversions you're 
> > doing and eliminate some of the risk of forgetting a bits-to-bytes 
> > conversion.  It looks like storing XLen as a CharUnits would also make this 
> > easier.
> XLen is defined throughout the RISC-V ISA and ABI documentation as the width 
> of the integer ('x') registers in bits. I've modified EmitVAArg to make use 
> of CharUnits to avoid a conversion - there don't seem to be any other 
> bit/byte conversions I can see.
Okay, I can accept that, thanks.


https://reviews.llvm.org/D40023



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

[PATCH] D41996: [clangd] Code completion uses Sema for NS-level things in the current file.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, ilya-biryukov, klimek.

To stay fast, it avoids deserializing anything outside the current file, by
disabling the LoadExternal code completion option added in r322377, when the
index is enabled.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41996

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -57,6 +57,7 @@
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Not;
+using ::testing::UnorderedElementsAre;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
   void
@@ -482,11 +483,11 @@
   Opts.Index = nullptr;
 
   auto Results = completions(R"cpp(
-  namespace ns { class No {}; }
+  namespace ns { class Local {}; }
   void f() { ns::^ }
   )cpp",
  Opts);
-  EXPECT_THAT(Results.items, Has("No"));
+  EXPECT_THAT(Results.items, Has("Local"));
 }
 
 TEST(CompletionTest, StaticAndDynamicIndex) {
@@ -514,13 +515,13 @@
   Opts.Index = I.get();
 
   auto Results = completions(R"cpp(
-  namespace ns { class No {}; }
+  namespace ns { class Local {}; }
   void f() { ns::^ }
   )cpp",
  Opts);
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
   EXPECT_THAT(Results.items, Has("foo", CompletionItemKind::Function));
-  EXPECT_THAT(Results.items, Not(Has("No")));
+  EXPECT_THAT(Results.items, Has("Local"));
 }
 
 TEST(CompletionTest, IndexBasedWithFilter) {
@@ -561,6 +562,41 @@
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
 }
 
+TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true);
+
+  FS.Files[getVirtualTestFilePath("bar.h")] =
+  R"cpp(namespace ns { int preamble; })cpp";
+  auto File = getVirtualTestFilePath("foo.cpp");
+  Annotations Test(R"cpp(
+  #include "bar.h"
+  namespace ns { int local; }
+  void f() { ns::^ }
+  )cpp");
+  Server.addDocument(Context::empty(), File, Test.code()).wait();
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto WithoutIndex =
+  Server.codeComplete(Context::empty(), File, Test.point(), Opts)
+  .get()
+  .second.Value;
+  EXPECT_THAT(WithoutIndex.items,
+  UnorderedElementsAre(Named("local"), Named("preamble")));
+
+  auto I = simpleIndexFromSymbols({{"ns::index", index::SymbolKind::Variable}});
+  Opts.Index = I.get();
+  auto WithIndex =
+  Server.codeComplete(Context::empty(), File, Test.point(), Opts)
+  .get()
+  .second.Value;
+  EXPECT_THAT(WithIndex.items,
+  UnorderedElementsAre(Named("local"), Named("index")));
+}
+
 TEST(CompletionTest, ASTIndexMultiFile) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -644,8 +644,10 @@
   Result.IncludeGlobals = IncludeGlobals;
   Result.IncludeBriefComments = IncludeBriefComments;
 
-  // Enable index-based code completion when Index is provided.
-  Result.IncludeNamespaceLevelDecls = !Index;
+  // When an is used, Sema is responsible for completing the main file,
+  // the index can provide results from the preamble.
+  // Tell Sema not to deserialize the preamble to look for results.
+  Result.LoadExternal = !Index;
 
   return Result;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Sorry for the delay in getting back to this, but testing this by building it 
explicitly for N32 (I built a full N32 compiler + libunwind and modified the 
test setup py to compile for N322) and I'm getting crashes on the return path 
in _Unwind_Backtrace.

The problem occurs when saving the registers, as saving the (second I believe) 
last register clobbers the saved return address. I've seen crashes on MIPS64 
(eb). Qemu however is aborting at runtime, but I haven't had the time to trace 
the execution, and I haven't found a way yet to dump the locations of the 
shared  libraries at runtime to determine where the fault lies.

This was with GCC 4.9.2 (debian) on MIPS64 and our 2016.05-06 toolchain for 
qemu.


https://reviews.llvm.org/D39074



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

[PATCH] D41980: Add tests for llvm-bcanalyzer stream types

[Brian Gesiak via Phabricator via cfe-commits]

modocache added inline comments.



Comment at: test/PCH/include-stream-type.cpp:7
+// RUN: cp %S/Inputs/pragma-once2.h %t-dir
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %t-dir/pragma-once2-pch.h
+// RUN: llvm-bcanalyzer -dump %t | FileCheck %s

aprantl wrote:
> You may need to pass an explicit -fmodule-format=raw for this to work on 
> macOS. Did you test there?
Yup, I tested this on my macOS host machine and it seems to pass. Should add 
that option anyway, just in case?


Repository:
  rC Clang

https://reviews.llvm.org/D41980



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

[PATCH] D41980: Add tests for llvm-bcanalyzer stream types

[Adrian Prantl via Phabricator via cfe-commits]

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



Comment at: test/PCH/include-stream-type.cpp:7
+// RUN: cp %S/Inputs/pragma-once2.h %t-dir
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %t-dir/pragma-once2-pch.h
+// RUN: llvm-bcanalyzer -dump %t | FileCheck %s

modocache wrote:
> aprantl wrote:
> > You may need to pass an explicit -fmodule-format=raw for this to work on 
> > macOS. Did you test there?
> Yup, I tested this on my macOS host machine and it seems to pass. Should add 
> that option anyway, just in case?
Let's add it, just to be explicit and document the intent here.


Repository:
  rC Clang

https://reviews.llvm.org/D41980



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

[clang-tools-extra] r322379 - [clangd] Include debugging tags for both static and dynamic index. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Jan 12 09:09:49 2018
New Revision: 322379

URL: http://llvm.org/viewvc/llvm-project?rev=322379&view=rev
Log:
[clangd] Include debugging tags for both static and dynamic index. NFC

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=322379&r1=322378&r2=322379&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Jan 12 09:09:49 2018
@@ -671,10 +671,10 @@ CompletionList codeComplete(const Contex
 // engine).
 if (Opts.Index)
   completeWithIndex(Ctx, *Opts.Index, Contents, *CompletedName.SSInfo,
-CompletedName.Filter, &Results);
+CompletedName.Filter, &Results, 
/*DebuggingLabel=*/"D");
 if (Opts.StaticIndex)
   completeWithIndex(Ctx, *Opts.StaticIndex, Contents, 
*CompletedName.SSInfo,
-CompletedName.Filter, &Results, 
/*DebuggingLabel=*/"G");
+CompletedName.Filter, &Results, 
/*DebuggingLabel=*/"S");
   }
   return Results;
 }

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=322379&r1=322378&r2=322379&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Jan 12 
09:09:49 2018
@@ -526,8 +526,8 @@ TEST(CompletionTest, StaticAndDynamicInd
   void f() { ::ns::^ }
   )cpp",
  Opts);
-  EXPECT_THAT(Results.items, Contains(Labeled("[G]XYZ")));
-  EXPECT_THAT(Results.items, Contains(Labeled("foo")));
+  EXPECT_THAT(Results.items, Contains(Labeled("[S]XYZ")));
+  EXPECT_THAT(Results.items, Contains(Labeled("[D]foo")));
 }
 
 TEST(CompletionTest, SimpleIndexBased) {


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

[PATCH] D41809: Clang counterpart change for fuzzer FreeBSD support

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

In https://reviews.llvm.org/D41809#969475, @kimgr wrote:

> Typo in the commit title: buzzer :)


Changed ... just to follow-up the now accepted change of this one 
https://reviews.llvm.org/D41642


https://reviews.llvm.org/D41809



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

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

High Integrity C++ has the rule `17.5.1 Do not ignore the result of 
std::remove, std::remove_if or std::unique`. Do we want to add those to the 
preconfigured list?


https://reviews.llvm.org/D41655



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

[PATCH] D41996: [clangd] Code completion uses Sema for NS-level things in the current file.

[Haojian Wu via Phabricator via cfe-commits]

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

nice, LGTM.

By `r322377`, I think you mean `r322371`.

We also need to set `CollectorOpts.IndexMainFiles` to `false` in  
`FileIndex.cpp::indexAST` -- we don't need dynamic index to catch the symbols 
in current main file since sema does that for us now :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41996



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

[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/Decl.h:3529
+  /// Basic properties of non-trivial C structs.
+  bool HasStrongObjCPointer : 1;
+

Is it interesting to track all the individual reasons that a struct is 
non-trivial at the struct level, as opposed to (like we do with 
CXXDefinitionData) just tracking the four aggregate properties you've described 
below?



Comment at: include/clang/AST/Type.h:1098
+  /// and return the kind.
+  PrimitiveCopyKind isNonTrivialToPrimitiveDefaultInitialize() const;
+

I think this one should probably get its own enum.  It's not too hard to 
imagine types (maybe relative references, or something else that's 
address-sensitive?) that require non-trivial copy semantics but isn't 
non-trivial to default-initialize.



Comment at: include/clang/AST/Type.h:1108
+  /// move and return the kind.
+  PrimitiveCopyKind isNonTrivialToPrimitiveDestructiveMove() const;
+

You need to define what you mean by "destructive move":

- A C++-style move leaves the source in a still-initialized state, just 
potentially with a different value (such as nil for a __strong reference).  
This is what we would do when e.g. loading from an r-value reference in C++.  
It's also what we have to do when moving a __block variable to the heap, since 
there may still be pointers to the stack copy of the variable, and since the 
compiler will unconditionally attempt to destroy that stack copy when it goes 
out of scope.  Since the source still needs to be destroyed, a 
non-trivially-copyable type will probably always have to do non-trivial work to 
do this.  Because of that, a function for this query could reasonably just 
return PrimitiveCopyKind.

- A truly primitive destructive move is something that actually leaves the 
source uninitialized.  This is generally only possible in narrow circumstances 
where the compiler can guarantee that the previous value is never again going 
to be referenced, including to destroy it.  I don't know if you have a reason 
to track this at all (i.e. whether there are copies you want to perform with a 
destructive move in IRGen), but if you do, you should use a different return 
type, because many types that are non-trivial to "C++-style move" are trivial 
to "destructively move": for example, __strong references are trivial to 
destructively move.



Comment at: include/clang/AST/Type.h:1113
+  /// the kind.
+  PrimitiveCopyKind isNonTrivialToPrimitiveDestroy() const;
+

Is there a reason we need this in addition to isDestructedType()?  It seems to 
me that it's exactly the same except ruling out the possibility of a C++ 
destructor.



Comment at: include/clang/AST/Type.h:1137
+return isNonTrivialToPrimitiveDestroy() == PCK_Struct;
+  }
+

Are these four queries really important enough to provide API for them on 
QualType?



Comment at: include/clang/AST/Type.h:1148
+DK_objc_weak_lifetime,
+DK_c_struct_strong_field
   };

I don't think you want to refer to the fact that the C struct specifically 
contains a __strong field here.  As we add more reasons, would we create a new 
enumerator for each?  What if a struct is non-trivial for multiple reasons?  
Just say that it's a non-trivial C struct.



Comment at: lib/AST/ASTContext.cpp:5778
+  if (Ty.isNonTrivialToPrimitiveDestructiveMoveStruct() ||
+  Ty.isNonTrivialToPrimitiveDestroyStruct())
+return true;

I think you should use the non-struct-specific checks here.  In addition to 
being cleaner, it would also let you completely short-circuit the rest of this 
function in ARC mode.  In non-ARC mode, you do still have to check lifetime 
(only to see if it's __unsafe_unretained; the __strong and __weak cases would 
be unreachable), and you need the type-specific special cases.


https://reviews.llvm.org/D41228



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

[PATCH] D41963: [clang-tidy] Adding Fuchsia checker for thread local storage.

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 129647.
juliehockett marked 3 inline comments as done.
juliehockett added a comment.

Fixing comments


https://reviews.llvm.org/D41963

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/ThreadLocalCheck.cpp
  clang-tidy/fuchsia/ThreadLocalCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-thread-local.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-thread-local.cpp

Index: test/clang-tidy/fuchsia-thread-local.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-thread-local.cpp
@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy %s fuchsia-thread-local %t
+
+int main() {
+  thread_local int foo;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: thread local storage is disallowed
+  // CHECK-NEXT:  thread_local int foo;
+
+  extern thread_local int bar;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: thread local storage is disallowed
+  // CHECK-NEXT:  extern thread_local int bar;
+  int baz;
+  
+  return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -71,6 +71,7 @@
fuchsia-default-arguments
fuchsia-overloaded-operator
fuchsia-statically-constructed-objects
+   fuchsia-thread-local
fuchsia-virtual-inheritance
google-build-explicit-make-pair
google-build-namespaces
Index: docs/clang-tidy/checks/fuchsia-thread-local.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-thread-local.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - fuchsia-thread-local
+
+fuchsia-thread-local
+
+
+Warns if thread storage duration is used.
+
+For example, using ``thread_local`` or ``extern thread_local`` is not allowed:
+
+.. code-block:: c++
+
+  thread_local int foo;   // Warning
+  extern thread_local int bar;// Warning
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -64,6 +64,11 @@
   object is statically initialized with a ``constexpr`` constructor or has no 
   explicit constructor.
 
+- New `fuchsia-thread-local
+  `_ check
+
+  Warns if thread storage duration is used.
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/fuchsia/ThreadLocalCheck.h
===
--- /dev/null
+++ clang-tidy/fuchsia/ThreadLocalCheck.h
@@ -0,0 +1,35 @@
+//===--- ThreadLocalCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_THREAD_LOCAL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_THREAD_LOCAL_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+/// Thread storage duration is disallowed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-thread-local.html
+class ThreadLocalCheck : public ClangTidyCheck {
+public:
+  ThreadLocalCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_THREAD_LOCAL_H
Index: clang-tidy/fuchsia/ThreadLocalCheck.cpp
===
--- /dev/null
+++ clang-tidy/fuchsia/ThreadLocalCheck.cpp
@@ -0,0 +1,32 @@
+//===--- ThreadLocalCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ThreadLocalCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+void ThreadLocalCheck::registerMatchers(MatchFinder *Finder) {
+  // Using thread storage duration is disallowed.
+  Finder->addMatcher(varDecl(hasThreadStorageDuration()).bind("de

[PATCH] D41998: [clang-tidy] Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts.

[Samuel Benzaquen via Phabricator via cfe-commits]

sbenza created this revision.
sbenza added a reviewer: hokein.
Herald added subscribers: cfe-commits, xazax.hun, klimek.

Expand readability-redundant-smartptr-get to understand implicit converions to 
bool in more contexts.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41998

Files:
  clang-tidy/readability/RedundantSmartptrGetCheck.cpp
  test/clang-tidy/readability-redundant-smartptr-get.cpp

Index: test/clang-tidy/readability-redundant-smartptr-get.cpp
===
--- test/clang-tidy/readability-redundant-smartptr-get.cpp
+++ test/clang-tidy/readability-redundant-smartptr-get.cpp
@@ -9,13 +9,15 @@
   T& operator*() const;
   T* operator->() const;
   T* get() const;
+  explicit operator bool() const noexcept;
 };
 
 template 
 struct shared_ptr {
   T& operator*() const;
   T* operator->() const;
   T* get() const;
+  explicit operator bool() const noexcept;
 };
 
 }  // namespace std
@@ -28,6 +30,7 @@
   Bar* operator->();
   Bar& operator*();
   Bar* get();
+  explicit operator bool() const;
 };
 struct int_ptr {
   int* get();
@@ -110,6 +113,23 @@
   // CHECK-MESSAGES: uu.get() == nullptr;
   // CHECK-FIXES: bool bb = uu == nullptr;
 
+  if (up->get());
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+  // CHECK-MESSAGES: if (up->get());
+  // CHECK-FIXES: if (*up);
+  if ((uu.get()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+  // CHECK-MESSAGES: if ((uu.get()));
+  // CHECK-FIXES: if ((uu));
+  bb = !ss->get();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant get() call
+  // CHECK-MESSAGES: bb = !ss->get();
+  // CHECK-FIXES: bb = !*ss;
+  bb = u.get() ? true : false;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+  // CHECK-MESSAGES: bb = u.get() ? true : false;
+  // CHECK-FIXES: bb = u ? true : false;
+
   bb = nullptr != ss->get();
   // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: redundant get() call
   // CHECK-MESSAGES: nullptr != ss->get();
@@ -146,10 +166,6 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant get() call
   // CHECK-MESSAGES: if (NULL == x.get());
   // CHECK-FIXES: if (NULL == x);
-  if (x.get());
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
-  // CHECK-MESSAGES: if (x.get());
-  // CHECK-FIXES: if (x);
 }
 
 void Negative() {
@@ -175,4 +191,6 @@
 
   int_ptr ip;
   bool bb = ip.get() == nullptr;
+  bb = !ip.get();
+  bb = ip.get() ? true : false;
 }
Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===
--- clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -18,6 +18,7 @@
 namespace readability {
 
 namespace {
+
 internal::Matcher callToGet(const internal::Matcher &OnClass) {
   return cxxMemberCallExpr(
  on(expr(anyOf(hasType(OnClass),
@@ -51,6 +52,20 @@
   unaryOperator(hasOperatorName("*"),
 hasUnaryOperand(callToGet(QuacksLikeASmartptr))),
   Callback);
+
+  // Catch '!ptr.get()'
+  const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(recordDecl(
+  QuacksLikeASmartptr, has(cxxConversionDecl(returns(booleanType()));
+  Finder->addMatcher(
+  unaryOperator(hasOperatorName("!"), hasUnaryOperand(CallToGetAsBool)),
+  Callback);
+
+  // Catch 'if(ptr.get())'
+  Finder->addMatcher(ifStmt(hasCondition(CallToGetAsBool)), Callback);
+
+  // Catch 'ptr.get() ? X : Y'
+  Finder->addMatcher(conditionalOperator(hasCondition(CallToGetAsBool)),
+ Callback);
 }
 
 void registerMatchersForGetEquals(MatchFinder *Finder,
@@ -72,11 +87,6 @@
  hasEitherOperand(callToGet(IsAKnownSmartptr))),
   Callback);
 
-  // Matches against if(ptr.get())
-  Finder->addMatcher(
-  ifStmt(hasCondition(ignoringImpCasts(callToGet(IsAKnownSmartptr,
-  Callback);
-
   // FIXME: Match and fix if (l.get() == r.get()).
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r322382 - [WebAssembly] Support -stdlib=libc++ switch

[Sam Clegg via cfe-commits]

Author: sbc
Date: Fri Jan 12 09:54:49 2018
New Revision: 322382

URL: http://llvm.org/viewvc/llvm-project?rev=322382&view=rev
Log:
[WebAssembly] Support -stdlib=libc++ switch

Referenced implementation from Fuchsia and Darwin Toolchain.
Still only support CST_Libcxx.  Now checks that the argument
is really '-stdlib=libc++', and display error.

Also, now will pass -lc++ and -lc++abi to the linker.

Patch by Patrick Cheng!

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

Added:
cfe/trunk/test/Driver/wasm-toolchain.cpp   (with props)
Modified:
cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
cfe/trunk/lib/Driver/ToolChains/WebAssembly.h

Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp?rev=322382&r1=322381&r2=322382&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp Fri Jan 12 09:54:49 2018
@@ -11,6 +11,7 @@
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
 
@@ -117,6 +118,12 @@ ToolChain::RuntimeLibType WebAssembly::G
 }
 
 ToolChain::CXXStdlibType WebAssembly::GetCXXStdlibType(const ArgList &Args) 
const {
+  if (Arg *A = Args.getLastArg(options::OPT_stdlib_EQ)) {
+StringRef Value = A->getValue();
+if (Value != "libc++")
+  getDriver().Diag(diag::err_drv_invalid_stdlib_name)
+  << A->getAsString(Args);
+  }
   return ToolChain::CST_Libcxx;
 }
 
@@ -134,6 +141,19 @@ void WebAssembly::AddClangCXXStdlibInclu
  getDriver().SysRoot + "/include/c++/v1");
 }
 
+void WebAssembly::AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+  llvm::opt::ArgStringList &CmdArgs) const 
{
+
+  switch (GetCXXStdlibType(Args)) {
+  case ToolChain::CST_Libcxx:
+CmdArgs.push_back("-lc++");
+CmdArgs.push_back("-lc++abi");
+break;
+  case ToolChain::CST_Libstdcxx:
+llvm_unreachable("invalid stdlib name");
+  }
+}
+
 std::string WebAssembly::getThreadModel() const {
   // The WebAssembly MVP does not yet support threads; for now, use the
   // "single" threading model, which lowers atomics to non-atomic operations.

Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.h?rev=322382&r1=322381&r2=322382&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.h Fri Jan 12 09:54:49 2018
@@ -62,6 +62,8 @@ private:
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const override;
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs) const override;
   std::string getThreadModel() const override;
 
   const char *getDefaultLinker() const override {

Added: cfe/trunk/test/Driver/wasm-toolchain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/wasm-toolchain.cpp?rev=322382&view=auto
==
--- cfe/trunk/test/Driver/wasm-toolchain.cpp (added)
+++ cfe/trunk/test/Driver/wasm-toolchain.cpp Fri Jan 12 09:54:49 2018
@@ -0,0 +1,36 @@
+// A basic clang -cc1 command-line. WebAssembly is somewhat special in
+// enabling -ffunction-sections, -fdata-sections, and -fvisibility=hidden by
+// default.
+
+// RUN: %clang++ %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
2>&1 | FileCheck -check-prefix=CC1 %s
+// CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} 
"-fvisibility" "hidden" {{.*}} "-ffunction-sections" "-fdata-sections"
+
+// Ditto, but ensure that a user -fno-function-sections disables the
+// default -ffunction-sections.
+
+// RUN: %clang++ %s -### -target wasm32-unknown-unknown -fno-function-sections 
2>&1 | FileCheck -check-prefix=NO_FUNCTION_SECTIONS %s
+// NO_FUNCTION_SECTIONS-NOT: function-sections
+
+// Ditto, but ensure that a user -fno-data-sections disables the
+// default -fdata-sections.
+
+// RUN: %clang++ %s -### -target wasm32-unknown-unknown -fno-data-sections 
2>&1 | FileCheck -check-prefix=NO_DATA_SECTIONS %s
+// NO_DATA_SECTIONS-NOT: data-sections
+
+// Ditto, but ensure that a user -fvisibility=default disables the default
+// -fvisibility=hidden.
+
+// RUN: %clang++ %s -### -target wasm32-unknown-unknown -fvisibility=default 
2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
+// FVISIBILITY_DEFAULT-NOT: hidden
+
+// A basic C++ link command-line.
+
+// RUN: %clang++ -### -no-canonical-prefixes -target wasm32-unknown-unknow

[PATCH] D41937: [WebAssembly] supports -stdlib=libc++ switch

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC322382: [WebAssembly] Support -stdlib=libc++ switch 
(authored by sbc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41937?vs=129572&id=129649#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41937

Files:
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.cpp


Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -11,6 +11,7 @@
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
 
@@ -117,6 +118,12 @@
 }
 
 ToolChain::CXXStdlibType WebAssembly::GetCXXStdlibType(const ArgList &Args) 
const {
+  if (Arg *A = Args.getLastArg(options::OPT_stdlib_EQ)) {
+StringRef Value = A->getValue();
+if (Value != "libc++")
+  getDriver().Diag(diag::err_drv_invalid_stdlib_name)
+  << A->getAsString(Args);
+  }
   return ToolChain::CST_Libcxx;
 }
 
@@ -134,6 +141,19 @@
  getDriver().SysRoot + "/include/c++/v1");
 }
 
+void WebAssembly::AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+  llvm::opt::ArgStringList &CmdArgs) const 
{
+
+  switch (GetCXXStdlibType(Args)) {
+  case ToolChain::CST_Libcxx:
+CmdArgs.push_back("-lc++");
+CmdArgs.push_back("-lc++abi");
+break;
+  case ToolChain::CST_Libstdcxx:
+llvm_unreachable("invalid stdlib name");
+  }
+}
+
 std::string WebAssembly::getThreadModel() const {
   // The WebAssembly MVP does not yet support threads; for now, use the
   // "single" threading model, which lowers atomics to non-atomic operations.
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -62,6 +62,8 @@
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const override;
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs) const override;
   std::string getThreadModel() const override;
 
   const char *getDefaultLinker() const override {
Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -0,0 +1,36 @@
+// A basic clang -cc1 command-line. WebAssembly is somewhat special in
+// enabling -ffunction-sections, -fdata-sections, and -fvisibility=hidden by
+// default.
+
+// RUN: %clang++ %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
2>&1 | FileCheck -check-prefix=CC1 %s
+// CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} 
"-fvisibility" "hidden" {{.*}} "-ffunction-sections" "-fdata-sections"
+
+// Ditto, but ensure that a user -fno-function-sections disables the
+// default -ffunction-sections.
+
+// RUN: %clang++ %s -### -target wasm32-unknown-unknown -fno-function-sections 
2>&1 | FileCheck -check-prefix=NO_FUNCTION_SECTIONS %s
+// NO_FUNCTION_SECTIONS-NOT: function-sections
+
+// Ditto, but ensure that a user -fno-data-sections disables the
+// default -fdata-sections.
+
+// RUN: %clang++ %s -### -target wasm32-unknown-unknown -fno-data-sections 
2>&1 | FileCheck -check-prefix=NO_DATA_SECTIONS %s
+// NO_DATA_SECTIONS-NOT: data-sections
+
+// Ditto, but ensure that a user -fvisibility=default disables the default
+// -fvisibility=hidden.
+
+// RUN: %clang++ %s -### -target wasm32-unknown-unknown -fvisibility=default 
2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
+// FVISIBILITY_DEFAULT-NOT: hidden
+
+// A basic C++ link command-line.
+
+// RUN: %clang++ -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK %s
+// LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" 
"-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
+// A basic C++ link command-line with optimization.
+
+// RUN: %clang++ -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck 
-check-prefix=LINK_OPT %s
+// LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"


Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssemb

[PATCH] D41788: [DeclPrinter] Fix two cases that crash clang -ast-print.

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

@arphaman: ping.


https://reviews.llvm.org/D41788



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

[PATCH] D41996: [clangd] Code completion uses Sema for NS-level things in the current file.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D41996#974751, @hokein wrote:

> nice, LGTM.
>
> By `r322377`, I think you mean `r322371`.


Done.

> We also need to set `CollectorOpts.IndexMainFiles` to `false` in  
> `FileIndex.cpp::indexAST` -- we don't need dynamic index to catch the symbols 
> in current main file since sema does that for us now :)

This would fix the problem. But it will break index-based go-to-definition for 
modified files, so I'd rather leave this at least for now, and pursue 
deduplicating symbols instead.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41996



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

[PATCH] D41999: Refactor handling of signext/zeroext in ABIArgInfo

[Alex Bradbury via Phabricator via cfe-commits]

asb created this revision.
asb added a reviewer: rjmccall.
Herald added subscribers: cfe-commits, arichardson.

As @rjmccall suggested in https://reviews.llvm.org/D40023, we can get rid of 
ABIInfo::shouldSignExtUnsignedType (used to handle cases like the Mips calling 
convention where 32-bit integers are always sign extended regardless of the 
sign of the type) by adding a SignExt field to ABIArgInfo. In the common case, 
this new field is set automatically by ABIArgInfo::getExtend based on the sign 
of the type. For targets that want greater control, they can use 
ABIArgInfo::getSignExtend or ABIArgInfo::getZeroExtend when necessary. This 
change also cleans up logic in CGCall.cpp.

There is no functional change intended in this patch, and all tests pass 
unchanged. As noted in https://reviews.llvm.org/D40023, Mips might want to 
32-bit integer return types. A future patch might modify 
MipsABIInfo::classifyReturnType to use MipsABIInfo::extendType.


Repository:
  rC Clang

https://reviews.llvm.org/D41999

Files:
  include/clang/CodeGen/CGFunctionInfo.h
  lib/CodeGen/ABIInfo.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/TargetInfo.cpp

Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -201,10 +201,6 @@
   return false;
 }
 
-bool ABIInfo::shouldSignExtUnsignedType(QualType Ty) const {
-  return false;
-}
-
 LLVM_DUMP_METHOD void ABIArgInfo::dump() const {
   raw_ostream &OS = llvm::errs();
   OS << "(ABIArgInfo Kind=";
@@ -682,8 +678,8 @@
   if (const EnumType *EnumTy = Ty->getAs())
 Ty = EnumTy->getDecl()->getIntegerType();
 
-  return (Ty->isPromotableIntegerType() ?
-  ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+  return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty)
+: ABIArgInfo::getDirect());
 }
 
 ABIArgInfo DefaultABIInfo::classifyReturnType(QualType RetTy) const {
@@ -697,8 +693,8 @@
   if (const EnumType *EnumTy = RetTy->getAs())
 RetTy = EnumTy->getDecl()->getIntegerType();
 
-  return (RetTy->isPromotableIntegerType() ?
-  ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+  return (RetTy->isPromotableIntegerType() ? ABIArgInfo::getExtend(RetTy)
+   : ABIArgInfo::getDirect());
 }
 
 //===--===//
@@ -845,8 +841,8 @@
 return ABIArgInfo::getDirect();
   }
 
-  return (Ty->isPromotableIntegerType() ?
-  ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+  return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty)
+: ABIArgInfo::getDirect());
 }
 
 ABIArgInfo PNaClABIInfo::classifyReturnType(QualType RetTy) const {
@@ -861,8 +857,8 @@
   if (const EnumType *EnumTy = RetTy->getAs())
 RetTy = EnumTy->getDecl()->getIntegerType();
 
-  return (RetTy->isPromotableIntegerType() ?
-  ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+  return (RetTy->isPromotableIntegerType() ? ABIArgInfo::getExtend(RetTy)
+   : ABIArgInfo::getDirect());
 }
 
 /// IsX86_MMXType - Return true if this is an MMX type.
@@ -1403,8 +1399,8 @@
   if (const EnumType *EnumTy = RetTy->getAs())
 RetTy = EnumTy->getDecl()->getIntegerType();
 
-  return (RetTy->isPromotableIntegerType() ?
-  ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+  return (RetTy->isPromotableIntegerType() ? ABIArgInfo::getExtend(RetTy)
+   : ABIArgInfo::getDirect());
 }
 
 static bool isSSEVectorType(ASTContext &Context, QualType Ty) {
@@ -1676,8 +1672,8 @@
 
   if (Ty->isPromotableIntegerType()) {
 if (InReg)
-  return ABIArgInfo::getExtendInReg();
-return ABIArgInfo::getExtend();
+  return ABIArgInfo::getExtendInReg(Ty);
+return ABIArgInfo::getExtend(Ty);
   }
 
   if (InReg)
@@ -2865,8 +2861,8 @@
 if (const EnumType *EnumTy = Ty->getAs())
   Ty = EnumTy->getDecl()->getIntegerType();
 
-return (Ty->isPromotableIntegerType() ?
-ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty)
+  : ABIArgInfo::getDirect());
   }
 
   return getNaturalAlignIndirect(Ty);
@@ -2898,8 +2894,8 @@
 if (const EnumType *EnumTy = Ty->getAs())
   Ty = EnumTy->getDecl()->getIntegerType();
 
-return (Ty->isPromotableIntegerType() ?
-ABIArgInfo::getExtend() : ABIArgInfo::getDirect());
+return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend(Ty)
+  : ABIArgInfo::getDirect());
   }
 
   if (CGCXXABI::RecordArgABI RAA = getRecordArgABI(Ty, getCXXABI()))
@@ -3268,7 +3264,7 @@
 
   if (RetTy->isIntegralOrEnumerationType() &&
   RetTy->isPromotableIntegerType())
-return ABIArg

[PATCH] D40023: [RISCV] Implement ABI lowering

[Alex Bradbury via Phabricator via cfe-commits]

asb added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1937
+RetAttrs.addAttribute(llvm::Attribute::ZExt);
+}
 // FALL THROUGH

rjmccall wrote:
> asb wrote:
> > rjmccall wrote:
> > > I feel like a better design would be to record whether to do a sext or a 
> > > zext in the ABIArgInfo.  Add getSignExtend and getZeroExtend static 
> > > functions to ABIArgInfo and make getExtend a convenience function that 
> > > takes a QualType and uses its signedness.
> > I could see how that might be cleaner, but that's a larger refactoring 
> > that's going to touch a lot more code. Are you happy for this patch to 
> > stick with this more incremental change (applying the same sign-extension 
> > logic to return values as is used for arguments), and to leave your 
> > suggested refactoring for a future patch?
> I won't insist that you do it, but I don't think this refactor would be as 
> bad as you think.  Doing these refactors incrementally when we realize that 
> the existing infrastructure is failing us in some way is how we make sure 
> they actually happen.  Individual contributors rarely have any incentive to 
> ever do that "future patch".
I've submitted this refactoring in D41999.


https://reviews.llvm.org/D40023



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

[PATCH] D41999: Refactor handling of signext/zeroext in ABIArgInfo

[John McCall via Phabricator via cfe-commits]

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

Thanks, that looks great.  I appreciate you doing this.


Repository:
  rC Clang

https://reviews.llvm.org/D41999



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:1937
+RetAttrs.addAttribute(llvm::Attribute::ZExt);
+}
 // FALL THROUGH

asb wrote:
> rjmccall wrote:
> > asb wrote:
> > > rjmccall wrote:
> > > > I feel like a better design would be to record whether to do a sext or 
> > > > a zext in the ABIArgInfo.  Add getSignExtend and getZeroExtend static 
> > > > functions to ABIArgInfo and make getExtend a convenience function that 
> > > > takes a QualType and uses its signedness.
> > > I could see how that might be cleaner, but that's a larger refactoring 
> > > that's going to touch a lot more code. Are you happy for this patch to 
> > > stick with this more incremental change (applying the same sign-extension 
> > > logic to return values as is used for arguments), and to leave your 
> > > suggested refactoring for a future patch?
> > I won't insist that you do it, but I don't think this refactor would be as 
> > bad as you think.  Doing these refactors incrementally when we realize that 
> > the existing infrastructure is failing us in some way is how we make sure 
> > they actually happen.  Individual contributors rarely have any incentive to 
> > ever do that "future patch".
> I've submitted this refactoring in D41999.
Much appreciated, thanks!


https://reviews.llvm.org/D40023



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

[PATCH] D41809: Clang counterpart change for fuzzer FreeBSD support

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

Looks good, please improve the description of the commit (and not your 
authorship that will be preserved in the commit message).

Once accepted by another developer, I can commit it for you.


https://reviews.llvm.org/D41809



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

[PATCH] D42001: [Driver] Add "did you mean?" suggestions to -cc1as

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: v.g.vassilev, bruno.

In https://reviews.llvm.org/D41733, the driver was modified such that,
when a user provided a mispelled option such as `-hel`, it would
suggest a valid option with a nearby edit distance: "did you mean
'-help'?".

Add these suggestions to invocations of `clang -cc1as` as well.

Test Plan: `check-clang`


Repository:
  rC Clang

https://reviews.llvm.org/D42001

Files:
  test/Driver/unknown-arg.c
  tools/driver/cc1as_main.cpp


Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -181,7 +181,13 @@
 
   // Issue errors on unknown arguments.
   for (const Arg *A : Args.filtered(OPT_UNKNOWN)) {
-Diags.Report(diag::err_drv_unknown_argument) << A->getAsString(Args);
+auto ArgString = A->getAsString(Args);
+std::string Nearest;
+if (OptTbl->findNearest(ArgString, Nearest, IncludedFlagsBitmask) > 1)
+  Diags.Report(diag::err_drv_unknown_argument) << ArgString;
+else
+  Diags.Report(diag::err_drv_unknown_argument_with_suggestion)
+  << ArgString << Nearest;
 Success = false;
   }
 
Index: test/Driver/unknown-arg.c
===
--- test/Driver/unknown-arg.c
+++ test/Driver/unknown-arg.c
@@ -12,6 +12,8 @@
 // RUN: FileCheck %s --check-prefix=CL-ERROR-DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -c -Wno-unknown-argument -### -- %s 2>&1 
| \
 // RUN: FileCheck %s --check-prefix=SILENT
+// RUN: not %clang -cc1as -hell --version -debug-info-macros 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CC1AS-DID-YOU-MEAN
 
 // CHECK: error: unknown argument: '-cake-is-lie'
 // CHECK: error: unknown argument: '-%0'
@@ -41,6 +43,9 @@
 // CL-ERROR-DID-YOU-MEAN: error: unknown argument ignored in clang-cl '-helo' 
(did you mean '-help'?)
 // SILENT-NOT: error:
 // SILENT-NOT: warning:
+// CC1AS-DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'?
+// CC1AS-DID-YOU-MEAN: error: unknown argument '--version', did you mean 
'-version'?
+// CC1AS-DID-YOU-MEAN: error: unknown argument '-debug-info-macros', did you 
mean '-debug-info-macro'?
 
 
 // RUN: %clang -S %s -o %t.s  -Wunknown-to-clang-option 2>&1 | FileCheck 
--check-prefix=IGNORED %s


Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -181,7 +181,13 @@
 
   // Issue errors on unknown arguments.
   for (const Arg *A : Args.filtered(OPT_UNKNOWN)) {
-Diags.Report(diag::err_drv_unknown_argument) << A->getAsString(Args);
+auto ArgString = A->getAsString(Args);
+std::string Nearest;
+if (OptTbl->findNearest(ArgString, Nearest, IncludedFlagsBitmask) > 1)
+  Diags.Report(diag::err_drv_unknown_argument) << ArgString;
+else
+  Diags.Report(diag::err_drv_unknown_argument_with_suggestion)
+  << ArgString << Nearest;
 Success = false;
   }
 
Index: test/Driver/unknown-arg.c
===
--- test/Driver/unknown-arg.c
+++ test/Driver/unknown-arg.c
@@ -12,6 +12,8 @@
 // RUN: FileCheck %s --check-prefix=CL-ERROR-DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -c -Wno-unknown-argument -### -- %s 2>&1 | \
 // RUN: FileCheck %s --check-prefix=SILENT
+// RUN: not %clang -cc1as -hell --version -debug-info-macros 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CC1AS-DID-YOU-MEAN
 
 // CHECK: error: unknown argument: '-cake-is-lie'
 // CHECK: error: unknown argument: '-%0'
@@ -41,6 +43,9 @@
 // CL-ERROR-DID-YOU-MEAN: error: unknown argument ignored in clang-cl '-helo' (did you mean '-help'?)
 // SILENT-NOT: error:
 // SILENT-NOT: warning:
+// CC1AS-DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'?
+// CC1AS-DID-YOU-MEAN: error: unknown argument '--version', did you mean '-version'?
+// CC1AS-DID-YOU-MEAN: error: unknown argument '-debug-info-macros', did you mean '-debug-info-macro'?
 
 
 // RUN: %clang -S %s -o %t.s  -Wunknown-to-clang-option 2>&1 | FileCheck --check-prefix=IGNORED %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r322387 - [clangd] Code completion uses Sema for NS-level things in the current file.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Jan 12 10:30:08 2018
New Revision: 322387

URL: http://llvm.org/viewvc/llvm-project?rev=322387&view=rev
Log:
[clangd] Code completion uses Sema for NS-level things in the current file.

Summary:
To stay fast, it avoids deserializing anything outside the current file, by
disabling the LoadExternal code completion option added in r322377, when the
index is enabled.

Reviewers: hokein

Subscribers: klimek, ilya-biryukov, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=322387&r1=322386&r2=322387&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Jan 12 10:30:08 2018
@@ -642,8 +642,10 @@ clang::CodeCompleteOptions CodeCompleteO
   Result.IncludeGlobals = IncludeGlobals;
   Result.IncludeBriefComments = IncludeBriefComments;
 
-  // Enable index-based code completion when Index is provided.
-  Result.IncludeNamespaceLevelDecls = !Index;
+  // When an is used, Sema is responsible for completing the main file,
+  // the index can provide results from the preamble.
+  // Tell Sema not to deserialize the preamble to look for results.
+  Result.LoadExternal = !Index;
 
   return Result;
 }

Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=322387&r1=322386&r2=322387&view=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Fri Jan 12 10:30:08 2018
@@ -20,6 +20,8 @@ std::unique_ptr indexAST(AST
  std::shared_ptr PP,
  llvm::ArrayRef Decls) {
   SymbolCollector::Options CollectorOpts;
+  // Code completion gets main-file results from Sema.
+  // But we leave this option on because features like go-to-definition want 
it.
   CollectorOpts.IndexMainFiles = true;
   auto Collector = std::make_shared(std::move(CollectorOpts));
   Collector->setPreprocessor(std::move(PP));

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=322387&r1=322386&r2=322387&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Jan 12 
10:30:08 2018
@@ -57,6 +57,7 @@ using ::testing::Contains;
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Not;
+using ::testing::UnorderedElementsAre;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
   void
@@ -104,7 +105,7 @@ CompletionList completions(StringRef Tex
   /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
-  Server.addDocument(Context::empty(), File, Test.code());
+  Server.addDocument(Context::empty(), File, Test.code()).wait();
   auto CompletionList =
   Server.codeComplete(Context::empty(), File, Test.point(), Opts)
   .get()
@@ -506,11 +507,11 @@ TEST(CompletionTest, NoIndex) {
   Opts.Index = nullptr;
 
   auto Results = completions(R"cpp(
-  namespace ns { class No {}; }
+  namespace ns { class Local {}; }
   void f() { ns::^ }
   )cpp",
  Opts);
-  EXPECT_THAT(Results.items, Has("No"));
+  EXPECT_THAT(Results.items, Has("Local"));
 }
 
 TEST(CompletionTest, StaticAndDynamicIndex) {
@@ -538,13 +539,13 @@ TEST(CompletionTest, SimpleIndexBased) {
   Opts.Index = I.get();
 
   auto Results = completions(R"cpp(
-  namespace ns { class No {}; }
+  namespace ns { int local; }
   void f() { ns::^ }
   )cpp",
  Opts);
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
   EXPECT_THAT(Results.items, Has("foo", CompletionItemKind::Function));
-  EXPECT_THAT(Results.items, Not(Has("No")));
+  EXPECT_THAT(Results.items, Has("local"));
 }
 
 TEST(CompletionTest, IndexBasedWithFilter) {
@@ -585,6 +586,41 @@ TEST(CompletionTest, FullyQualifiedScope
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
 }
 
+TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, DiagConsumer, FS, getDefau

[PATCH] D41996: [clangd] Code completion uses Sema for NS-level things in the current file.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL322387: [clangd] Code completion uses Sema for NS-level 
things in the current file. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D41996?vs=129637&id=129659#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41996

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/clangd/index/FileIndex.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -642,8 +642,10 @@
   Result.IncludeGlobals = IncludeGlobals;
   Result.IncludeBriefComments = IncludeBriefComments;
 
-  // Enable index-based code completion when Index is provided.
-  Result.IncludeNamespaceLevelDecls = !Index;
+  // When an is used, Sema is responsible for completing the main file,
+  // the index can provide results from the preamble.
+  // Tell Sema not to deserialize the preamble to look for results.
+  Result.LoadExternal = !Index;
 
   return Result;
 }
Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp
@@ -20,6 +20,8 @@
  std::shared_ptr PP,
  llvm::ArrayRef Decls) {
   SymbolCollector::Options CollectorOpts;
+  // Code completion gets main-file results from Sema.
+  // But we leave this option on because features like go-to-definition want it.
   CollectorOpts.IndexMainFiles = true;
   auto Collector = std::make_shared(std::move(CollectorOpts));
   Collector->setPreprocessor(std::move(PP));
Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -57,6 +57,7 @@
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Not;
+using ::testing::UnorderedElementsAre;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
   void
@@ -104,7 +105,7 @@
   /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
-  Server.addDocument(Context::empty(), File, Test.code());
+  Server.addDocument(Context::empty(), File, Test.code()).wait();
   auto CompletionList =
   Server.codeComplete(Context::empty(), File, Test.point(), Opts)
   .get()
@@ -506,11 +507,11 @@
   Opts.Index = nullptr;
 
   auto Results = completions(R"cpp(
-  namespace ns { class No {}; }
+  namespace ns { class Local {}; }
   void f() { ns::^ }
   )cpp",
  Opts);
-  EXPECT_THAT(Results.items, Has("No"));
+  EXPECT_THAT(Results.items, Has("Local"));
 }
 
 TEST(CompletionTest, StaticAndDynamicIndex) {
@@ -538,13 +539,13 @@
   Opts.Index = I.get();
 
   auto Results = completions(R"cpp(
-  namespace ns { class No {}; }
+  namespace ns { int local; }
   void f() { ns::^ }
   )cpp",
  Opts);
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
   EXPECT_THAT(Results.items, Has("foo", CompletionItemKind::Function));
-  EXPECT_THAT(Results.items, Not(Has("No")));
+  EXPECT_THAT(Results.items, Has("local"));
 }
 
 TEST(CompletionTest, IndexBasedWithFilter) {
@@ -585,6 +586,41 @@
   EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class));
 }
 
+TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true);
+
+  FS.Files[getVirtualTestFilePath("bar.h")] =
+  R"cpp(namespace ns { int preamble; })cpp";
+  auto File = getVirtualTestFilePath("foo.cpp");
+  Annotations Test(R"cpp(
+  #include "bar.h"
+  namespace ns { int local; }
+  void f() { ns::^ }
+  )cpp");
+  Server.addDocument(Context::empty(), File, Test.code()).wait();
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto WithoutIndex =
+  Server.codeComplete(Context::empty(), File, Test.point(), Opts)
+  .get()
+  .second.Value;
+  EXPECT_THAT(WithoutIndex.items,
+  UnorderedElementsAre(Named("local"), Named("preamble")));
+
+  auto I = simpleIndexFromSymbols({{"ns::index", index::SymbolKind::Variable}});
+  Opts.Index = I.get();
+  auto WithIndex =
+  Server.codeComplete(Context::empty(), File, Test.point(), Opts)
+  .get()
+  .second

[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: include/clang/AST/Type.h:1148
+DK_objc_weak_lifetime,
+DK_c_struct_strong_field
   };

rjmccall wrote:
> I don't think you want to refer to the fact that the C struct specifically 
> contains a __strong field here.  As we add more reasons, would we create a 
> new enumerator for each?  What if a struct is non-trivial for multiple 
> reasons?  Just say that it's a non-trivial C struct.
I added an enumerator for DK_c_struct_strong_field since 
CodeGenFunction::needsEHCleanup distinguishes between `__weak` and `__strong` 
types. Is it not necessary to distinguish between a struct that has a `__weak` 
field and a struct that has a `__strong` field but not a `__weak` field? 


https://reviews.llvm.org/D41228



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

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[John Baldwin via Phabricator via cfe-commits]

bsdjhb added a comment.

To be clear, are you getting the failure running libunwind's test suite or your 
own test?  I've managed to get libunwind to cross-compile for me using GCC 
6.3.0 on FreeBSD for O32, N32, and N64, but only to build the library, not the 
tests.  I've been running a simple C++ test program (which is using a patched 
libunwind along with libc++ as it's C++ runtime) for testing.  The program uses 
_Unwind_Backtrace() as well as throws a couple of C++ exceptions with catch 
handlers that print out the values thrown.  If you are able to cross-build the 
libunwind tests and then run them under qemu I'd appreciate a pointer to get 
that working as I'd be happier running libunwind's tests than my own.


https://reviews.llvm.org/D39074



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

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

This was libunwind's test suite:

  Compiled test failed unexpectedly!
  
  Testing Time: 0.53s
  
  Failing Tests (1):
  libunwind :: libunwind_02.pass.cpp
  
Expected Passes: 3
Unexpected Failures: 1 

The hacky patch I used to test n32:

  --- a/test/libunwind/test/config.py
  +++ b/test/libunwind/test/config.py
  @@ -48,6 +48,8 @@ class Configuration(LibcxxConfiguration):
   # Stack unwinding tests need unwinding tables and these are not
   # generated by default on all Targets.
   self.cxx.compile_flags += ['-funwind-tables']
  +self.cxx.compile_flags += ['-mabi=n33']
  +self.cxx.link_flags += ['-mabi=n32']
   if not self.get_lit_bool('enable_threads', True):
   self.cxx.compile_flags += ['-D_LIBUNWIND_HAS_NO_THREADS']
   self.config.available_features.add('libunwind-no-threads')


https://reviews.llvm.org/D39074



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

[PATCH] D42001: [Driver] Add "did you mean?" suggestions to -cc1as

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D42001



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

r322390 - [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Fri Jan 12 10:54:35 2018
New Revision: 322390

URL: http://llvm.org/viewvc/llvm-project?rev=322390&view=rev
Log:
[Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.

Fix makes the loop in LexAngledStringLiteral more like the loops in
LexStringLiteral, LexCharConstant. When we skip a character after
backslash, we need to check if we reached the end of the file instead of
reading the next character unconditionally.

Discovered by OSS-Fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3832

rdar://problem/35572754

Reviewers: arphaman, kcc, rsmith, dexonsmith

Reviewed By: rsmith, dexonsmith

Subscribers: cfe-commits, rsmith, dexonsmith

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

Added:
cfe/trunk/test/Lexer/null-character-in-literal.c   (with props)
Modified:
cfe/trunk/lib/Lex/Lexer.cpp
cfe/trunk/unittests/Lex/LexerTest.cpp

Modified: cfe/trunk/lib/Lex/Lexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=322390&r1=322389&r2=322390&view=diff
==
--- cfe/trunk/lib/Lex/Lexer.cpp (original)
+++ cfe/trunk/lib/Lex/Lexer.cpp Fri Jan 12 10:54:35 2018
@@ -2009,18 +2009,21 @@ bool Lexer::LexAngledStringLiteral(Token
   const char *AfterLessPos = CurPtr;
   char C = getAndAdvanceChar(CurPtr, Result);
   while (C != '>') {
-// Skip escaped characters.
-if (C == '\\' && CurPtr < BufferEnd) {
-  // Skip the escaped character.
-  getAndAdvanceChar(CurPtr, Result);
-} else if (C == '\n' || C == '\r' || // Newline.
-   (C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
-   isCodeCompletionPoint(CurPtr-1 {
+// Skip escaped characters.  Escaped newlines will already be processed by
+// getAndAdvanceChar.
+if (C == '\\')
+  C = getAndAdvanceChar(CurPtr, Result);
+
+if (C == '\n' || C == '\r' || // Newline.
+(C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
+isCodeCompletionPoint(CurPtr-1 {
   // If the filename is unterminated, then it must just be a lone <
   // character.  Return this as such.
   FormTokenWithChars(Result, AfterLessPos, tok::less);
   return true;
-} else if (C == 0) {
+}
+
+if (C == 0) {
   NulCharacter = CurPtr-1;
 }
 C = getAndAdvanceChar(CurPtr, Result);

Added: cfe/trunk/test/Lexer/null-character-in-literal.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/null-character-in-literal.c?rev=322390&view=auto
==
Binary file - no diff available.

Propchange: cfe/trunk/test/Lexer/null-character-in-literal.c
--
svn:mime-type = application/octet-stream

Modified: cfe/trunk/unittests/Lex/LexerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=322390&r1=322389&r2=322390&view=diff
==
--- cfe/trunk/unittests/Lex/LexerTest.cpp (original)
+++ cfe/trunk/unittests/Lex/LexerTest.cpp Fri Jan 12 10:54:35 2018
@@ -475,6 +475,8 @@ TEST_F(LexerTest, GetBeginningOfTokenWit
 
 TEST_F(LexerTest, AvoidPastEndOfStringDereference) {
   EXPECT_TRUE(Lex("  //  \\\n").empty());
+  EXPECT_TRUE(Lex("#include <").empty());
+  EXPECT_TRUE(Lex("#include <\n").empty());
 }
 
 TEST_F(LexerTest, StringizingRasString) {


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

[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC322390: [Lex] Avoid out-of-bounds dereference in 
LexAngledStringLiteral. (authored by vsapsai, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41423?vs=129379&id=129666#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41423

Files:
  lib/Lex/Lexer.cpp
  test/Lexer/null-character-in-literal.c
  unittests/Lex/LexerTest.cpp


Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -2009,18 +2009,21 @@
   const char *AfterLessPos = CurPtr;
   char C = getAndAdvanceChar(CurPtr, Result);
   while (C != '>') {
-// Skip escaped characters.
-if (C == '\\' && CurPtr < BufferEnd) {
-  // Skip the escaped character.
-  getAndAdvanceChar(CurPtr, Result);
-} else if (C == '\n' || C == '\r' || // Newline.
-   (C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
-   isCodeCompletionPoint(CurPtr-1 {
+// Skip escaped characters.  Escaped newlines will already be processed by
+// getAndAdvanceChar.
+if (C == '\\')
+  C = getAndAdvanceChar(CurPtr, Result);
+
+if (C == '\n' || C == '\r' || // Newline.
+(C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
+isCodeCompletionPoint(CurPtr-1 {
   // If the filename is unterminated, then it must just be a lone <
   // character.  Return this as such.
   FormTokenWithChars(Result, AfterLessPos, tok::less);
   return true;
-} else if (C == 0) {
+}
+
+if (C == 0) {
   NulCharacter = CurPtr-1;
 }
 C = getAndAdvanceChar(CurPtr, Result);
Index: unittests/Lex/LexerTest.cpp
===
--- unittests/Lex/LexerTest.cpp
+++ unittests/Lex/LexerTest.cpp
@@ -475,6 +475,8 @@
 
 TEST_F(LexerTest, AvoidPastEndOfStringDereference) {
   EXPECT_TRUE(Lex("  //  \\\n").empty());
+  EXPECT_TRUE(Lex("#include <").empty());
+  EXPECT_TRUE(Lex("#include <\n").empty());
 }
 
 TEST_F(LexerTest, StringizingRasString) {


Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -2009,18 +2009,21 @@
   const char *AfterLessPos = CurPtr;
   char C = getAndAdvanceChar(CurPtr, Result);
   while (C != '>') {
-// Skip escaped characters.
-if (C == '\\' && CurPtr < BufferEnd) {
-  // Skip the escaped character.
-  getAndAdvanceChar(CurPtr, Result);
-} else if (C == '\n' || C == '\r' || // Newline.
-   (C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
-   isCodeCompletionPoint(CurPtr-1 {
+// Skip escaped characters.  Escaped newlines will already be processed by
+// getAndAdvanceChar.
+if (C == '\\')
+  C = getAndAdvanceChar(CurPtr, Result);
+
+if (C == '\n' || C == '\r' || // Newline.
+(C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
+isCodeCompletionPoint(CurPtr-1 {
   // If the filename is unterminated, then it must just be a lone <
   // character.  Return this as such.
   FormTokenWithChars(Result, AfterLessPos, tok::less);
   return true;
-} else if (C == 0) {
+}
+
+if (C == 0) {
   NulCharacter = CurPtr-1;
 }
 C = getAndAdvanceChar(CurPtr, Result);
Index: unittests/Lex/LexerTest.cpp
===
--- unittests/Lex/LexerTest.cpp
+++ unittests/Lex/LexerTest.cpp
@@ -475,6 +475,8 @@
 
 TEST_F(LexerTest, AvoidPastEndOfStringDereference) {
   EXPECT_TRUE(Lex("  //  \\\n").empty());
+  EXPECT_TRUE(Lex("#include <").empty());
+  EXPECT_TRUE(Lex("#include <\n").empty());
 }
 
 TEST_F(LexerTest, StringizingRasString) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41815: [clang-tidy] implement check for goto

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:20
+
+AST_MATCHER(GotoStmt, isForwardJumping) {
+

lebedev.ri wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > It would be nice to have it in standard ASTMatchers, i believe it will be 
> > > useful for `else-after-return` check.
> > > Though the ASTMatchers are stuck due to the doc-dumping script being 
> > > 'broken' (see D41455)
> > Yes. The GNU extension goto does not have ASTMatcher yet, so i will pack 
> > these together in a review. What do you think how long the ASTMatcher issue 
> > will be there? Maybe it could be done after that check lands?
> > Maybe it could be done after that check lands?
> 
> Yes, absolutely. I did not meant that as a blocker here.
On my todo list.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41815



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

[PATCH] D42004: [Driver] Suggest valid integrated tools

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: sepavloff, bkramer, phosek.

There are only two valid integrated Clang driver tools: `-cc1` and
`-cc1as`. If a user asks for an unknown tool, such as `-cc1asphalt`,
an error message is displayed to indicate that there is no such tool,
but the message doesn't indicate what the valid options are.

Include the valid options in the error message.

Test Plan: `check-clang`


Repository:
  rC Clang

https://reviews.llvm.org/D42004

Files:
  test/Driver/unknown-arg.c
  tools/driver/driver.cpp


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -311,7 +311,8 @@
 return cc1as_main(argv.slice(2), argv[0], GetExecutablePathVP);
 
   // Reject unknown tools.
-  llvm::errs() << "error: unknown integrated tool '" << Tool << "'\n";
+  llvm::errs() << "error: unknown integrated tool '" << Tool << "'. "
+   << "Valid tools include '-cc1' and '-cc1as'.\n";
   return 1;
 }
 
Index: test/Driver/unknown-arg.c
===
--- test/Driver/unknown-arg.c
+++ test/Driver/unknown-arg.c
@@ -12,6 +12,8 @@
 // RUN: FileCheck %s --check-prefix=CL-ERROR-DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -c -Wno-unknown-argument -### -- %s 2>&1 
| \
 // RUN: FileCheck %s --check-prefix=SILENT
+// RUN: not %clang -cc1asphalt -help 2>&1 | \
+// RUN: FileCheck %s --check-prefix=UNKNOWN-INTEGRATED
 
 // CHECK: error: unknown argument: '-cake-is-lie'
 // CHECK: error: unknown argument: '-%0'
@@ -41,6 +43,7 @@
 // CL-ERROR-DID-YOU-MEAN: error: unknown argument ignored in clang-cl '-helo' 
(did you mean '-help'?)
 // SILENT-NOT: error:
 // SILENT-NOT: warning:
+// UNKNOWN-INTEGRATED: error: unknown integrated tool 'asphalt'. Valid tools 
include '-cc1' and '-cc1as'.
 
 
 // RUN: %clang -S %s -o %t.s  -Wunknown-to-clang-option 2>&1 | FileCheck 
--check-prefix=IGNORED %s


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -311,7 +311,8 @@
 return cc1as_main(argv.slice(2), argv[0], GetExecutablePathVP);
 
   // Reject unknown tools.
-  llvm::errs() << "error: unknown integrated tool '" << Tool << "'\n";
+  llvm::errs() << "error: unknown integrated tool '" << Tool << "'. "
+   << "Valid tools include '-cc1' and '-cc1as'.\n";
   return 1;
 }
 
Index: test/Driver/unknown-arg.c
===
--- test/Driver/unknown-arg.c
+++ test/Driver/unknown-arg.c
@@ -12,6 +12,8 @@
 // RUN: FileCheck %s --check-prefix=CL-ERROR-DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -c -Wno-unknown-argument -### -- %s 2>&1 | \
 // RUN: FileCheck %s --check-prefix=SILENT
+// RUN: not %clang -cc1asphalt -help 2>&1 | \
+// RUN: FileCheck %s --check-prefix=UNKNOWN-INTEGRATED
 
 // CHECK: error: unknown argument: '-cake-is-lie'
 // CHECK: error: unknown argument: '-%0'
@@ -41,6 +43,7 @@
 // CL-ERROR-DID-YOU-MEAN: error: unknown argument ignored in clang-cl '-helo' (did you mean '-help'?)
 // SILENT-NOT: error:
 // SILENT-NOT: warning:
+// UNKNOWN-INTEGRATED: error: unknown integrated tool 'asphalt'. Valid tools include '-cc1' and '-cc1as'.
 
 
 // RUN: %clang -S %s -o %t.s  -Wunknown-to-clang-option 2>&1 | FileCheck --check-prefix=IGNORED %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r322393 - [OPENMP] Replace calls of getAssociatedStmt().

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Jan 12 11:39:11 2018
New Revision: 322393

URL: http://llvm.org/viewvc/llvm-project?rev=322393&view=rev
Log:
[OPENMP] Replace calls of getAssociatedStmt().

getAssociatedStmt() returns the outermost captured statement for the
OpenMP directive. It may return incorrect region in case of combined
constructs. Reworked the code to reduce the number of calls of
getAssociatedStmt() and used getInnermostCapturedStmt() and
getCapturedStmt() functions instead.
In case of firstprivate variables it may lead to an extra allocas
generation for private copies even if the variable is passed by value
into outlined function and could be used directly as private copy.

Modified:
cfe/trunk/include/clang/AST/StmtOpenMP.h
cfe/trunk/lib/AST/StmtPrinter.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/OpenMP/nvptx_target_teams_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp

Modified: cfe/trunk/include/clang/AST/StmtOpenMP.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtOpenMP.h?rev=322393&r1=322392&r2=322393&view=diff
==
--- cfe/trunk/include/clang/AST/StmtOpenMP.h (original)
+++ cfe/trunk/include/clang/AST/StmtOpenMP.h Fri Jan 12 11:39:11 2018
@@ -222,6 +222,25 @@ public:
 llvm_unreachable("Incorrect RegionKind specified for directive.");
   }
 
+  /// Get innermost captured statement for the construct.
+  CapturedStmt *getInnermostCapturedStmt() {
+assert(hasAssociatedStmt() && getAssociatedStmt() &&
+   "Must have associated statement.");
+SmallVector CaptureRegions;
+getOpenMPCaptureRegions(CaptureRegions, getDirectiveKind());
+assert(!CaptureRegions.empty() &&
+   "At least one captured statement must be provided.");
+auto *CS = cast(getAssociatedStmt());
+for (unsigned Level = CaptureRegions.size(); Level > 1; --Level)
+  CS = cast(CS->getCapturedStmt());
+return CS;
+  }
+
+  const CapturedStmt *getInnermostCapturedStmt() const {
+return const_cast(this)
+->getInnermostCapturedStmt();
+  }
+
   OpenMPDirectiveKind getDirectiveKind() const { return Kind; }
 
   static bool classof(const Stmt *S) {
@@ -903,9 +922,8 @@ public:
   }
   const Stmt *getBody() const {
 // This relies on the loop form is already checked by Sema.
-const Stmt *Body = getAssociatedStmt()->IgnoreContainers(true);
-while(const auto *CS = dyn_cast(Body))
-  Body = CS->getCapturedStmt();
+const Stmt *Body =
+getInnermostCapturedStmt()->getCapturedStmt()->IgnoreContainers();
 Body = cast(Body)->getBody();
 for (unsigned Cnt = 1; Cnt < CollapsedNum; ++Cnt) {
   Body = Body->IgnoreContainers();

Modified: cfe/trunk/lib/AST/StmtPrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtPrinter.cpp?rev=322393&r1=322392&r2=322393&view=diff
==
--- cfe/trunk/lib/AST/StmtPrinter.cpp (original)
+++ cfe/trunk/lib/AST/StmtPrinter.cpp Fri Jan 12 11:39:11 2018
@@ -1034,12 +1034,8 @@ void StmtPrinter::PrintOMPExecutableDire
   OS << ' ';
 }
   OS << "\n";
-  if (S->hasAssociatedStmt() && S->getAssociatedStmt() && !ForceNoStmt) {
-assert(isa(S->getAssociatedStmt()) &&
-   "Expected captured statement!");
-Stmt *CS = cast(S->getAssociatedStmt())->getCapturedStmt();
-PrintStmt(CS);
-  }
+  if (!ForceNoStmt && S->hasAssociatedStmt())
+PrintStmt(S->getInnermostCapturedStmt()->getCapturedStmt());
 }
 
 void StmtPrinter::VisitOMPParallelDirective(OMPParallelDirective *Node) {
@@ -1142,7 +1138,7 @@ void StmtPrinter::VisitOMPFlushDirective
 
 void StmtPrinter::VisitOMPOrderedDirective(OMPOrderedDirective *Node) {
   Indent() << "#pragma omp ordered ";
-  PrintOMPExecutableDirective(Node);
+  PrintOMPExecutableDirective(Node, Node->hasClausesOfKind());
 }
 
 void StmtPrinter::VisitOMPAtomicDirective(OMPAtomicDirective *Node) {

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=322393&r1=322392&r2=322393&view=diff
==

Re: r322390 - [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.

[Volodymyr Sapsai via cfe-commits]

Hans, I am nominating this change to be merged into 6.0.0 release branch.

Thanks,
Volodymyr

> On Jan 12, 2018, at 10:54, Volodymyr Sapsai via cfe-commits 
>  wrote:
> 
> Author: vsapsai
> Date: Fri Jan 12 10:54:35 2018
> New Revision: 322390
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=322390&view=rev
> Log:
> [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
> 
> Fix makes the loop in LexAngledStringLiteral more like the loops in
> LexStringLiteral, LexCharConstant. When we skip a character after
> backslash, we need to check if we reached the end of the file instead of
> reading the next character unconditionally.
> 
> Discovered by OSS-Fuzz:
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=3832
> 
> rdar://problem/35572754
> 
> Reviewers: arphaman, kcc, rsmith, dexonsmith
> 
> Reviewed By: rsmith, dexonsmith
> 
> Subscribers: cfe-commits, rsmith, dexonsmith
> 
> Differential Revision: https://reviews.llvm.org/D41423
> 
> Added:
>cfe/trunk/test/Lexer/null-character-in-literal.c   (with props)
> Modified:
>cfe/trunk/lib/Lex/Lexer.cpp
>cfe/trunk/unittests/Lex/LexerTest.cpp
> 
> Modified: cfe/trunk/lib/Lex/Lexer.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=322390&r1=322389&r2=322390&view=diff
> ==
> --- cfe/trunk/lib/Lex/Lexer.cpp (original)
> +++ cfe/trunk/lib/Lex/Lexer.cpp Fri Jan 12 10:54:35 2018
> @@ -2009,18 +2009,21 @@ bool Lexer::LexAngledStringLiteral(Token
>   const char *AfterLessPos = CurPtr;
>   char C = getAndAdvanceChar(CurPtr, Result);
>   while (C != '>') {
> -// Skip escaped characters.
> -if (C == '\\' && CurPtr < BufferEnd) {
> -  // Skip the escaped character.
> -  getAndAdvanceChar(CurPtr, Result);
> -} else if (C == '\n' || C == '\r' || // Newline.
> -   (C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
> -   isCodeCompletionPoint(CurPtr-1 {
> +// Skip escaped characters.  Escaped newlines will already be processed 
> by
> +// getAndAdvanceChar.
> +if (C == '\\')
> +  C = getAndAdvanceChar(CurPtr, Result);
> +
> +if (C == '\n' || C == '\r' || // Newline.
> +(C == 0 && (CurPtr-1 == BufferEnd ||  // End of file.
> +isCodeCompletionPoint(CurPtr-1 {
>   // If the filename is unterminated, then it must just be a lone <
>   // character.  Return this as such.
>   FormTokenWithChars(Result, AfterLessPos, tok::less);
>   return true;
> -} else if (C == 0) {
> +}
> +
> +if (C == 0) {
>   NulCharacter = CurPtr-1;
> }
> C = getAndAdvanceChar(CurPtr, Result);
> 
> Added: cfe/trunk/test/Lexer/null-character-in-literal.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/null-character-in-literal.c?rev=322390&view=auto
> ==
> Binary file - no diff available.
> 
> Propchange: cfe/trunk/test/Lexer/null-character-in-literal.c
> --
>svn:mime-type = application/octet-stream
> 
> Modified: cfe/trunk/unittests/Lex/LexerTest.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=322390&r1=322389&r2=322390&view=diff
> ==
> --- cfe/trunk/unittests/Lex/LexerTest.cpp (original)
> +++ cfe/trunk/unittests/Lex/LexerTest.cpp Fri Jan 12 10:54:35 2018
> @@ -475,6 +475,8 @@ TEST_F(LexerTest, GetBeginningOfTokenWit
> 
> TEST_F(LexerTest, AvoidPastEndOfStringDereference) {
>   EXPECT_TRUE(Lex("  //  \\\n").empty());
> +  EXPECT_TRUE(Lex("#include <").empty());
> +  EXPECT_TRUE(Lex("#include <\n").empty());
> }
> 
> TEST_F(LexerTest, StringizingRasString) {
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[PATCH] D42005: [docs] Use monospace for PCH option flags

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: sepavloff, aaron.ballman.

Use monospace for option flags in the PCH section, instead of the
italics that were being used previously.

I believe these used to be links, for which single backticks would
have been appropriate, but since they were un-link-ified in
https://reviews.llvm.org/rL275560, I believe monospace is now more
appropriate, and so two backticks are needed.

Test Plan:
Build the `docs-clang-html` target and confirm the options are rendered
using monospace font.


Repository:
  rC Clang

https://reviews.llvm.org/D42005

Files:
  docs/UsersManual.rst


Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -1075,7 +1075,7 @@
 Building a relocatable precompiled header requires two additional
 arguments. First, pass the ``--relocatable-pch`` flag to indicate that
 the resulting PCH file should be relocatable. Second, pass
-`-isysroot /path/to/build`, which makes all includes for your library
+``-isysroot /path/to/build``, which makes all includes for your library
 relative to the build directory. For example:
 
 .. code-block:: console
@@ -1085,9 +1085,9 @@
 When loading the relocatable PCH file, the various headers used in the
 PCH file are found from the system header root. For example, ``mylib.h``
 can be found in ``/usr/include/mylib.h``. If the headers are installed
-in some other system root, the `-isysroot` option can be used provide
+in some other system root, the ``-isysroot`` option can be used provide
 a different system root from which the headers will be based. For
-example, `-isysroot /Developer/SDKs/MacOSX10.4u.sdk` will look for
+example, ``-isysroot /Developer/SDKs/MacOSX10.4u.sdk`` will look for
 ``mylib.h`` in ``/Developer/SDKs/MacOSX10.4u.sdk/usr/include/mylib.h``.
 
 Relocatable precompiled headers are intended to be used in a limited


Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -1075,7 +1075,7 @@
 Building a relocatable precompiled header requires two additional
 arguments. First, pass the ``--relocatable-pch`` flag to indicate that
 the resulting PCH file should be relocatable. Second, pass
-`-isysroot /path/to/build`, which makes all includes for your library
+``-isysroot /path/to/build``, which makes all includes for your library
 relative to the build directory. For example:
 
 .. code-block:: console
@@ -1085,9 +1085,9 @@
 When loading the relocatable PCH file, the various headers used in the
 PCH file are found from the system header root. For example, ``mylib.h``
 can be found in ``/usr/include/mylib.h``. If the headers are installed
-in some other system root, the `-isysroot` option can be used provide
+in some other system root, the ``-isysroot`` option can be used provide
 a different system root from which the headers will be based. For
-example, `-isysroot /Developer/SDKs/MacOSX10.4u.sdk` will look for
+example, ``-isysroot /Developer/SDKs/MacOSX10.4u.sdk`` will look for
 ``mylib.h`` in ``/Developer/SDKs/MacOSX10.4u.sdk/usr/include/mylib.h``.
 
 Relocatable precompiled headers are intended to be used in a limited
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Um, I now appear to be getting different results for running under QEMU doing 
it the proper way. I was previously rebuilding the failing test by hand and 
running under qemu. I don't believe I changed anything important, I'll have to 
take a longer look.

If you define an Executor for libunwind you can run the testsuite under QEMU, 
automagically.

I have:

  LIBUNWIND_EXECUTOR   
PrefixExecutor(['/home/sdardis/bin/qemu-mipsn32.sh'],LocalExecutor())

/home/sdardis/bin/qemu-mips32.sh for is a simple bash script:

  #!/bin/bash
  ~/mips-mti-linux-gnu/2016.05-06/bin/qemu-mipsn32 -L 
/home/snd-local/mips-mti-linux-gnu/2016.05-06/sysroot/mips-r2-hard/ -E 
LD_LIBRARY_PATH=/home/snd-local/mips-mti-linux-gnu/2016.05-06/mips-mti-linux-gnu/lib/mips-r2-hard/lib32/
 "$@"

Hope this helps.

Thanks for all the work you're putting into this.


https://reviews.llvm.org/D39074



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

[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy

[András Leitereg via Phabricator via cfe-commits]

leanil updated this revision to Diff 129676.
leanil marked an inline comment as done.
leanil added a comment.

Measure array size in bytes instead of elements.


https://reviews.llvm.org/D41384

Files:
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  test/Analysis/security-syntax-checks.m


Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -146,6 +146,16 @@
   strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as 
it does not provide bounding of the memory buffer. Replace unbounded copy 
functions with analogous functions that support length arguments such as 
'strlcpy'. CWE-119}}
 }
 
+void test_strcpy_2() {
+  char x[4];
+  strcpy(x, "abcd"); //expected-warning{{Call to function 'strcpy' is insecure 
as it does not provide bounding of the memory buffer. Replace unbounded copy 
functions with analogous functions that support length arguments such as 
'strlcpy'. CWE-119}}
+}
+
+void test_strcpy_safe() {
+  char x[5];
+  strcpy(x, "abcd");
+}
+
 //===--===
 // strcat()
 //===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,17 @@
   if (!checkCall_strCommon(CE, FD))
 return;
 
+  const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+  if (const auto *DeclRef = dyn_cast(Target))
+if (const auto *Array = dyn_cast(DeclRef->getType())) {
+  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  if (const auto *String = dyn_cast(Source)) {
+if (ArraySize >= String->getLength() + 1)
+  return;
+  }
+}
+
   // Issue a warning.
   PathDiagnosticLocation CELoc =
 PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);


Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -146,6 +146,16 @@
   strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119}}
 }
 
+void test_strcpy_2() {
+  char x[4];
+  strcpy(x, "abcd"); //expected-warning{{Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119}}
+}
+
+void test_strcpy_safe() {
+  char x[5];
+  strcpy(x, "abcd");
+}
+
 //===--===
 // strcat()
 //===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,17 @@
   if (!checkCall_strCommon(CE, FD))
 return;
 
+  const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+  if (const auto *DeclRef = dyn_cast(Target))
+if (const auto *Array = dyn_cast(DeclRef->getType())) {
+  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  if (const auto *String = dyn_cast(Source)) {
+if (ArraySize >= String->getLength() + 1)
+  return;
+  }
+}
+
   // Issue a warning.
   PathDiagnosticLocation CELoc =
 PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41815: [clang-tidy] implement check for goto

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 129677.
JonasToth marked 8 inline comments as done.
JonasToth added a comment.

- address review comments
- add better documentation with code examples


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41815

Files:
  clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
  clang-tidy/cppcoreguidelines/AvoidGotoCheck.h
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-avoid-goto.cpp

Index: test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t
+
+void noop() {}
+
+int main() {
+  noop();
+  goto jump_to_me;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+  noop();
+
+jump_to_me:;
+
+jump_backwards:;
+  noop();
+  goto jump_backwards;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+  // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not jump backwards with 'goto'
+  // CHECK-MESSAGES: [[@LINE-5]]:1: note: label defined here
+
+  // Test the GNU extension https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
+some_label:;
+  void *dynamic_label = &&some_label;
+
+  // FIXME: Not detected
+  goto *dynamic_label;
+  // CHECK MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+}
+
+void forward_jump_out_nested_loop() {
+  for (int i = 0; i < 10; ++i) {
+noop();
+for (int j = 0; j < 10; ++j) {
+  noop();
+  if (i + j > 10)
+goto early_exit1;
+}
+noop();
+  }
+
+  for (int i = 0; i < 10; ++i) {
+noop();
+while (true) {
+  noop();
+  if (i > 5)
+goto early_exit1;
+}
+noop();
+  }
+  // Jumping further results in error, because the variable declaration would
+  // be skipped.
+early_exit1:;
+
+  int i = 0;
+  while (true) {
+noop();
+while (true) {
+  noop();
+  if (i > 5)
+goto early_exit2;
+  i++;
+}
+noop();
+  }
+
+  while (true) {
+noop();
+for (int j = 0; j < 10; ++j) {
+  noop();
+  if (j > 5)
+goto early_exit2;
+}
+noop();
+  }
+
+early_exit2:;
+}
+
+void jump_out_backwards() {
+
+before_the_loop:
+  noop();
+
+  for (int i = 0; i < 10; ++i) {
+for (int j = 0; j < 10; ++j) {
+  if (i * j > 80)
+goto before_the_loop;
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: do not jump backwards with 'goto'
+  // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here
+}
+  }
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -52,6 +52,7 @@
cert-msc30-c (redirects to cert-msc50-cpp) 
cert-msc50-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
+   cppcoreguidelines-avoid-goto
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-no-malloc
Index: docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-goto
+
+cppcoreguidelines-avoid-goto
+
+
+The usage of ``goto`` for control flow is error prone and should be replaced
+with looping constructs. Only forward jumps in nested loops are accepted.
+
+This check implements `ES.76 `_ 
+from the CppCoreGuidelines and 
+`6.3.1 from High Integrity C++ `_.
+
+For more information on why to avoid programming 
+with ``goto`` you can read the famous paper `A Case against the GO TO Statement. `_.
+
+The check diagnoses ``goto`` for backward jumps in every language mode. These
+should be replaced with `C/C++` looping constructs.
+
+.. code-block:: c++
+
+  // Bad, handwritten for loop.
+  int i = 0;
+  // Jump label for the loop
+  loop_start:
+  do_some_operation();
+
+  if (i < 100) {
+++i;
+goto loop_start;
+  }
+
+  // Better
+  for(int i = 0; i < 100; ++i)
+do_some_operation();
+
+Modern C++ needs ``goto`` only to jump out of nested loops.
+
+.. code-block:: c++
+
+  for(int i = 0; i < 100; ++i) {
+for(int j = 0; j < 100; ++j) {
+  if (i * j > 500)
+

[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy

[András Leitereg via Phabricator via cfe-commits]

leanil marked 2 inline comments as done.
leanil added a comment.

In https://reviews.llvm.org/D41384#973851, @NoQ wrote:

> Do you have commit access or should someone else commit it for you?


I don't have, please commit it.




Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr())) {
+  unsigned long long ArraySize = Array->getSize().getLimitedValue();

NoQ wrote:
> This can be simplified to `const auto *Array = 
> DeclRef->getType()->getAs()`.
> `.getTypePtr()` is almost always redundant because of the fancy 
> `operator->()` on `QualType`.
Using `getAs` yielded: 
> error: static assertion failed: ArrayType cannot be used with getAs!




https://reviews.llvm.org/D41384



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

[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr())) {
+  unsigned long long ArraySize = Array->getSize().getLimitedValue();

leanil wrote:
> NoQ wrote:
> > This can be simplified to `const auto *Array = 
> > DeclRef->getType()->getAs()`.
> > `.getTypePtr()` is almost always redundant because of the fancy 
> > `operator->()` on `QualType`.
> Using `getAs` yielded: 
> > error: static assertion failed: ArrayType cannot be used with getAs!
> 
> 
Whoops, yeah, right, array types are the rare exception. It should be 
`ASTContext.getAsConstantArrayType()`, see the docs for `getAsArrayType()` for 
some explanation of why it was made this way. I guess we don't really care 
about these aspects, so your code is fine :)


https://reviews.llvm.org/D41384



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

[PATCH] D41999: Refactor handling of signext/zeroext in ABIArgInfo

[Alex Bradbury via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC322396: Refactor handling of signext/zeroext in ABIArgInfo 
(authored by asb, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D41999

Files:
  include/clang/CodeGen/CGFunctionInfo.h
  lib/CodeGen/ABIInfo.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/TargetInfo.cpp

Index: include/clang/CodeGen/CGFunctionInfo.h
===
--- include/clang/CodeGen/CGFunctionInfo.h
+++ include/clang/CodeGen/CGFunctionInfo.h
@@ -95,6 +95,7 @@
   bool SRetAfterThis : 1;   // isIndirect()
   bool InReg : 1;   // isDirect() || isExtend() || isIndirect()
   bool CanBeFlattened: 1;   // isDirect()
+  bool SignExt : 1; // isExtend()
 
   bool canHavePaddingType() const {
 return isDirect() || isExtend() || isIndirect() || isExpand();
@@ -133,15 +134,38 @@
 AI.setInReg(true);
 return AI;
   }
-  static ABIArgInfo getExtend(llvm::Type *T = nullptr) {
+
+  static ABIArgInfo getSignExtend(QualType Ty, llvm::Type *T = nullptr) {
+assert(Ty->isIntegralOrEnumerationType() && "Unexpected QualType");
 auto AI = ABIArgInfo(Extend);
 AI.setCoerceToType(T);
 AI.setPaddingType(nullptr);
 AI.setDirectOffset(0);
+AI.setSignExt(true);
 return AI;
   }
-  static ABIArgInfo getExtendInReg(llvm::Type *T = nullptr) {
-auto AI = getExtend(T);
+
+  static ABIArgInfo getZeroExtend(QualType Ty, llvm::Type *T = nullptr) {
+assert(Ty->isIntegralOrEnumerationType() && "Unexpected QualType");
+auto AI = ABIArgInfo(Extend);
+AI.setCoerceToType(T);
+AI.setPaddingType(nullptr);
+AI.setDirectOffset(0);
+AI.setSignExt(false);
+return AI;
+  }
+
+  // ABIArgInfo will record the argument as being extended based on the sign
+  // of its type.
+  static ABIArgInfo getExtend(QualType Ty, llvm::Type *T = nullptr) {
+assert(Ty->isIntegralOrEnumerationType() && "Unexpected QualType");
+if (Ty->hasSignedIntegerRepresentation())
+  return getSignExtend(Ty, T);
+return getZeroExtend(Ty, T);
+  }
+
+  static ABIArgInfo getExtendInReg(QualType Ty, llvm::Type *T = nullptr) {
+auto AI = getExtend(Ty, T);
 AI.setInReg(true);
 return AI;
   }
@@ -254,6 +278,15 @@
 DirectOffset = Offset;
   }
 
+  bool isSignExt() const {
+assert(isExtend() && "Invalid kind!");
+return SignExt;
+  }
+  void setSignExt(bool SExt) {
+assert(isExtend() && "Invalid kind!");
+SignExt = SExt;
+  }
+
   llvm::Type *getPaddingType() const {
 return (canHavePaddingType() ? PaddingType : nullptr);
   }
Index: lib/CodeGen/ABIInfo.h
===
--- lib/CodeGen/ABIInfo.h
+++ lib/CodeGen/ABIInfo.h
@@ -108,8 +108,6 @@
 virtual bool isHomogeneousAggregateSmallEnough(const Type *Base,
uint64_t Members) const;
 
-virtual bool shouldSignExtUnsignedType(QualType Ty) const;
-
 bool isHomogeneousAggregate(QualType Ty, const Type *&Base,
 uint64_t &Members) const;
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1925,9 +1925,9 @@
   const ABIArgInfo &RetAI = FI.getReturnInfo();
   switch (RetAI.getKind()) {
   case ABIArgInfo::Extend:
-if (RetTy->hasSignedIntegerRepresentation())
+if (RetAI.isSignExt())
   RetAttrs.addAttribute(llvm::Attribute::SExt);
-else if (RetTy->hasUnsignedIntegerRepresentation())
+else
   RetAttrs.addAttribute(llvm::Attribute::ZExt);
 LLVM_FALLTHROUGH;
   case ABIArgInfo::Direct:
@@ -2006,14 +2006,10 @@
 // sense to do it here because parameters are so messed up.
 switch (AI.getKind()) {
 case ABIArgInfo::Extend:
-  if (ParamType->isSignedIntegerOrEnumerationType())
+  if (AI.isSignExt())
 Attrs.addAttribute(llvm::Attribute::SExt);
-  else if (ParamType->isUnsignedIntegerOrEnumerationType()) {
-if (getTypes().getABIInfo().shouldSignExtUnsignedType(ParamType))
-  Attrs.addAttribute(llvm::Attribute::SExt);
-else
-  Attrs.addAttribute(llvm::Attribute::ZExt);
-  }
+  else
+Attrs.addAttribute(llvm::Attribute::ZExt);
   LLVM_FALLTHROUGH;
 case ABIArgInfo::Direct:
   if (ArgNo == 0 && FI.isChainCall())
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -201,10 +201,6 @@
   return false;
 }
 
-bool ABIInfo::shouldSignExtUnsignedType(QualType Ty) const {
-  return false;
-}
-
 LLVM_DUMP_METHOD void ABIArgInfo::dump() const {
   raw_ostream &OS = llvm::errs();
   OS << "(ABIArgInfo Kind=";
@@ -682,8 +678,8 @@
   if (const EnumType *EnumTy = Ty->getAs())
 Ty = EnumTy->getDecl()->getIntegerType();
 
-  return (Ty->isProm

[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:1148
+DK_objc_weak_lifetime,
+DK_c_struct_strong_field
   };

ahatanak wrote:
> rjmccall wrote:
> > I don't think you want to refer to the fact that the C struct specifically 
> > contains a __strong field here.  As we add more reasons, would we create a 
> > new enumerator for each?  What if a struct is non-trivial for multiple 
> > reasons?  Just say that it's a non-trivial C struct.
> I added an enumerator for DK_c_struct_strong_field since 
> CodeGenFunction::needsEHCleanup distinguishes between `__weak` and `__strong` 
> types. Is it not necessary to distinguish between a struct that has a 
> `__weak` field and a struct that has a `__strong` field but not a `__weak` 
> field? 
Oh, that's right.

I... hmm.  The problem is that that's really a special-case behavior, and we're 
designing a more general feature.  If we try to handle the special case in the 
first patch, we'll end up trivializing the general case, and that will permeate 
the design in unfortunate ways.

So I recommend that we *not* treat them separately right now; just emit 
cleanups for them unconditionally.  You're still planning to add support for 
`__weak` properties in a follow-up patch, right?  Once that's in, it will make 
more sense to carve out the `__strong`-only behavior as a special case.


https://reviews.llvm.org/D41228



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

[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

[Szabolcs Nagy via Phabricator via cfe-commits]

nsz added a comment.

In https://reviews.llvm.org/D40673#973638, @efriedma wrote:

> > as this patch is committed clang is broken (cannot use glibc headers)
>
> This patch was supposed to *fix* compatibility with the glibc headers.  If it 
> doesn't, we should clearly revert it... but we need to figure out what we 
> need to do to be compatible first.
>
> From what I can see on this thread, we *must* define _Float128 on x86-64 
> Linux, and we *must not* define _Float128 for any other glibc target.  Is 
> that correct?  Or does it depend on the glibc version?


it is not clear to me from the original bug report what "fedora 27 workloads" 
break because of the lack of _Float128 type on x86.  The glibc headers refer to 
_Float128, but normal include should not break unless the compiler claims to be 
>=gcc-7 or somebody explicitly requested the _Float128 support.

if clang defines _Float128 then the headers "work" on x86 as in simple math.h 
include is not broken, but some macro definitions will use the f128 const 
suffix (e.g. FLT128_MAX) which won't cause much breakage now but in the future 
users will want to know whether they can use these macros or not.

so either clang have to introduce all these features that are used by glibc 
together or provide ways for users (and glibc) to figure out what is supported 
and what isn't.

on non-x86 targets a glibc fix can solve the problem such that they "work" on 
the same level as x86, i.e. no immediate build breakage on most code, but some 
features are not supported. i think that's the right way forward, but it's not 
clear if anybody has time to do the header changes in glibc before release (it 
has to be tested on several targets).

if glibc is released as is today then those non-x86 targets don't want a 
_Float128 definition in clang-6 that breaks any math.h include on a glibc-2.27 
system.

> (We have a bit of time before the 6.0 release, so we can adjust the behavior 
> here to make it work.  We probably don't want to try to add full _Float128 
> support on the branch, though.)




Repository:
  rC Clang

https://reviews.llvm.org/D40673



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

[PATCH] D41815: [clang-tidy] implement check for goto

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 5 inline comments as done.
JonasToth added inline comments.



Comment at: docs/ReleaseNotes.rst:63
+
+  The usage of ``goto`` has been discouraged for a long time and is diagnosed
+  with this check.

Eugene.Zelenko wrote:
> I think will be good idea to reformulate this statement and its copy in 
> documentation. //diagnosed with this check// is tautological for any check.
Reformulated, is it ok now?



Comment at: docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst:6-7
+
+The usage of ``goto`` has been discouraged for a long time and is diagnosed
+with this check.
+

aaron.ballman wrote:
> This doesn't really help the user understand what's bad about goto or why it 
> should be diagnosed. You should expound a bit here.
Is it better now?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41815



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[Alex Bradbury via Phabricator via cfe-commits]

asb updated this revision to Diff 129683.
asb marked 8 inline comments as done.
asb added a comment.

Rebase after ABIArgInfo signext/zeroext refactoring 
https://reviews.llvm.org/D41999 / https://reviews.llvm.org/rL322396. We no 
longer need to modify CGCall.cpp for unsigned 32-bit return values to be sign 
extended as required by the RV64 ABI.


https://reviews.llvm.org/D40023

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/riscv32-abi.c
  test/CodeGen/riscv64-abi.c
  test/Driver/riscv32-toolchain.c
  test/Driver/riscv64-toolchain.c

Index: test/Driver/riscv64-toolchain.c
===
--- test/Driver/riscv64-toolchain.c
+++ test/Driver/riscv64-toolchain.c
@@ -42,3 +42,50 @@
 
 // CHECK: @align_vl = global i32 8
 int align_vl = __alignof(va_list);
+
+// Check types
+
+// CHECK: define zeroext i8 @check_char()
+char check_char() { return 0; }
+
+// CHECK: define signext i16 @check_short()
+short check_short() { return 0; }
+
+// CHECK: define signext i32 @check_int()
+int check_int() { return 0; }
+
+// CHECK: define signext i32 @check_wchar_t()
+int check_wchar_t() { return 0; }
+
+// CHECK: define i64 @check_long()
+long check_long() { return 0; }
+
+// CHECK: define i64 @check_longlong()
+long long check_longlong() { return 0; }
+
+// CHECK: define zeroext i8 @check_uchar()
+unsigned char check_uchar() { return 0; }
+
+// CHECK: define zeroext i16 @check_ushort()
+unsigned short check_ushort() { return 0; }
+
+// CHECK: define signext i32 @check_uint()
+unsigned int check_uint() { return 0; }
+
+// CHECK: define i64 @check_ulong()
+unsigned long check_ulong() { return 0; }
+
+// CHECK: define i64 @check_ulonglong()
+unsigned long long check_ulonglong() { return 0; }
+
+// CHECK: define i64 @check_size_t()
+size_t check_size_t() { return 0; }
+
+// CHECK: define float @check_float()
+float check_float() { return 0; }
+
+// CHECK: define double @check_double()
+double check_double() { return 0; }
+
+// CHECK: define fp128 @check_longdouble()
+long double check_longdouble() { return 0; }
Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -73,3 +73,50 @@
 
 // CHECK: @align_vl = global i32 4
 int align_vl = __alignof(va_list);
+
+// Check types
+
+// CHECK: zeroext i8 @check_char()
+char check_char() { return 0; }
+
+// CHECK: define signext i16 @check_short()
+short check_short() { return 0; }
+
+// CHECK: define i32 @check_int()
+int check_int() { return 0; }
+
+// CHECK: define i32 @check_wchar_t()
+int check_wchar_t() { return 0; }
+
+// CHECK: define i32 @check_long()
+long check_long() { return 0; }
+
+// CHECK: define i64 @check_longlong()
+long long check_longlong() { return 0; }
+
+// CHECK: define zeroext i8 @check_uchar()
+unsigned char check_uchar() { return 0; }
+
+// CHECK: define zeroext i16 @check_ushort()
+unsigned short check_ushort() { return 0; }
+
+// CHECK: define i32 @check_uint()
+unsigned int check_uint() { return 0; }
+
+// CHECK: define i32 @check_ulong()
+unsigned long check_ulong() { return 0; }
+
+// CHECK: define i64 @check_ulonglong()
+unsigned long long check_ulonglong() { return 0; }
+
+// CHECK: define i32 @check_size_t()
+size_t check_size_t() { return 0; }
+
+// CHECK: define float @check_float()
+float check_float() { return 0; }
+
+// CHECK: define double @check_double()
+double check_double() { return 0; }
+
+// CHECK: define fp128 @check_longdouble()
+long double check_longdouble() { return 0; }
Index: test/CodeGen/riscv64-abi.c
===
--- /dev/null
+++ test/CodeGen/riscv64-abi.c
@@ -0,0 +1,425 @@
+// RUN: %clang_cc1 -triple riscv64 -emit-llvm %s -o - | FileCheck %s
+
+#include 
+#include 
+
+// CHECK-LABEL: define void @f_void()
+void f_void(void) {}
+
+// Scalar arguments and return values smaller than the word size are extended
+// according to the sign of their type, up to 32 bits
+
+// CHECK-LABEL: define zeroext i1 @f_scalar_0(i1 zeroext %x)
+_Bool f_scalar_0(_Bool x) { return x; }
+
+// CHECK-LABEL: define signext i8 @f_scalar_1(i8 signext %x)
+int8_t f_scalar_1(int8_t x) { return x; }
+
+// CHECK-LABEL: define zeroext i8 @f_scalar_2(i8 zeroext %x)
+uint8_t f_scalar_2(uint8_t x) { return x; }
+
+// CHECK-LABEL: define signext i32 @f_scalar_3(i32 signext %x)
+uint32_t f_scalar_3(int32_t x) { return x; }
+
+// CHECK-LABEL: define i64 @f_scalar_4(i64 %x)
+int64_t f_scalar_4(int64_t x) { return x; }
+
+// CHECK-LABEL: define float @f_fp_scalar_1(float %x)
+float f_fp_scalar_1(float x) { return x; }
+
+// CHECK-LABEL: define double @f_fp_scalar_2(double %x)
+double f_fp_scalar_2(double x) { return x; }
+
+// CHECK-LABEL: define fp128 @f_fp_scalar_3(fp128 %x)
+long double f_fp_scalar_3(long double x) { return x; }
+
+// Empty structs or unions are ignored.
+
+struct empty_s {};
+
+// CHECK-LABEL: define void @f_agg_empt

[PATCH] D41815: [clang-tidy] implement check for goto

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 129684.
JonasToth added a comment.

- simplified the code and merged diagnostics


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41815

Files:
  clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
  clang-tidy/cppcoreguidelines/AvoidGotoCheck.h
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-avoid-goto.cpp

Index: test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
@@ -0,0 +1,89 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t
+
+void noop() {}
+
+int main() {
+  noop();
+  goto jump_to_me;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+  noop();
+
+jump_to_me:;
+
+jump_backwards:;
+  noop();
+  goto jump_backwards;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+  // CHECK-MESSAGES: [[@LINE-4]]:1: note: label defined here
+
+  // Test the GNU extension https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
+some_label:;
+  void *dynamic_label = &&some_label;
+
+  // FIXME: Not detected
+  goto *dynamic_label;
+}
+
+void forward_jump_out_nested_loop() {
+  for (int i = 0; i < 10; ++i) {
+noop();
+for (int j = 0; j < 10; ++j) {
+  noop();
+  if (i + j > 10)
+goto early_exit1;
+}
+noop();
+  }
+
+  for (int i = 0; i < 10; ++i) {
+noop();
+while (true) {
+  noop();
+  if (i > 5)
+goto early_exit1;
+}
+noop();
+  }
+  // Jumping further results in error, because the variable declaration would
+  // be skipped.
+early_exit1:;
+
+  int i = 0;
+  while (true) {
+noop();
+while (true) {
+  noop();
+  if (i > 5)
+goto early_exit2;
+  i++;
+}
+noop();
+  }
+
+  while (true) {
+noop();
+for (int j = 0; j < 10; ++j) {
+  noop();
+  if (j > 5)
+goto early_exit2;
+}
+noop();
+  }
+
+early_exit2:;
+}
+
+void jump_out_backwards() {
+
+before_the_loop:
+  noop();
+
+  for (int i = 0; i < 10; ++i) {
+for (int j = 0; j < 10; ++j) {
+  if (i * j > 80)
+goto before_the_loop;
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: avoid using 'goto' for flow control
+  // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here
+}
+  }
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -52,6 +52,7 @@
cert-msc30-c (redirects to cert-msc50-cpp) 
cert-msc50-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
+   cppcoreguidelines-avoid-goto
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-no-malloc
Index: docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-goto
+
+cppcoreguidelines-avoid-goto
+
+
+The usage of ``goto`` for control flow is error prone and should be replaced
+with looping constructs. Only forward jumps in nested loops are accepted.
+
+This check implements `ES.76 `_ 
+from the CppCoreGuidelines and 
+`6.3.1 from High Integrity C++ `_.
+
+For more information on why to avoid programming 
+with ``goto`` you can read the famous paper `A Case against the GO TO Statement. `_.
+
+The check diagnoses ``goto`` for backward jumps in every language mode. These
+should be replaced with `C/C++` looping constructs.
+
+.. code-block:: c++
+
+  // Bad, handwritten for loop.
+  int i = 0;
+  // Jump label for the loop
+  loop_start:
+  do_some_operation();
+
+  if (i < 100) {
+++i;
+goto loop_start;
+  }
+
+  // Better
+  for(int i = 0; i < 100; ++i)
+do_some_operation();
+
+Modern C++ needs ``goto`` only to jump out of nested loops.
+
+.. code-block:: c++
+
+  for(int i = 0; i < 100; ++i) {
+for(int j = 0; j < 100; ++j) {
+  if (i * j > 500)
+goto early_exit;
+}
+  }
+
+  early_exit:
+  some_operation();
+
+For C++11 or higher all other uses of ``goto`` are diagnosed.
Index: docs/ReleaseNotes.rst
==

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

@sbenza and/or @klimek did you have time to address the stackoverflow caused 
from the ASTMatchers?

I forgot to add the link to the original break: 
http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/5470/consoleFull#17462642768254eaf0-7326-4999-85b0-388101f2d404


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 129686.
JonasToth added a comment.

- get up to date to 7.0


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,472 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch without labels
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if/else statement
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+break;
+  }
+
+  bool Flag = false;
+  switch (Flag) {
+// CHECK-MESSAGES:[[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case true:
+break;
+  }
+
+  switch (Flag) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // This `switch` will create a frontend warning from '-Wswitch-bool' but is
+  // ok for this check.
+  switch (Flag) {
+  case true:
+break;
+  case false:
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  //
+  switch (c) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  ca

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

[Timothy VanSlyke via Phabricator via cfe-commits]

tvanslyke updated this revision to Diff 129685.
tvanslyke added a comment.

I went ahead and just pulled it out to a small inline member function and added 
it to __copy_assign_alloc().  Removed some other redundancies.


https://reviews.llvm.org/D41976

Files:
  string


Index: string
===
--- string
+++ string
@@ -1407,24 +1407,30 @@
   
__alloc_traits::propagate_on_container_copy_assignment::value>());}
 
 _LIBCPP_INLINE_VISIBILITY
+void __clear_and_shrink() 
+{
+clear();
+if(__is_long())
+__alloc_traits::deallocate(__alloc(), __get_long_pointer(), 
capacity() + 1);
+} 
+
+_LIBCPP_INLINE_VISIBILITY
 void __copy_assign_alloc(const basic_string& __str, true_type)
 {
 if (__alloc() == __str.__alloc())
 __alloc() = __str.__alloc();
 else
 {
 if (!__str.__is_long())
 {
-clear();
-shrink_to_fit();
+__clear_and_shrink();
 __alloc() = __str.__alloc();
 }
 else
 {
 allocator_type __a = __str.__alloc();
 pointer __p = __alloc_traits::allocate(__a, 
__str.__get_long_cap());
-clear();
-shrink_to_fit();
+__clear_and_shrink();
 __alloc() = _VSTD::move(__a);
 __set_long_pointer(__p);
 __set_long_cap(__str.__get_long_cap());
@@ -2102,8 +2108,7 @@
 _NOEXCEPT_(is_nothrow_move_assignable::value)
 #endif
 {
-clear();
-shrink_to_fit();
+__clear_and_shrink();
 __r_.first() = __str.__r_.first();
 __move_assign_alloc(__str);
 __str.__zero();


Index: string
===
--- string
+++ string
@@ -1407,24 +1407,30 @@
   __alloc_traits::propagate_on_container_copy_assignment::value>());}
 
 _LIBCPP_INLINE_VISIBILITY
+void __clear_and_shrink() 
+{
+clear();
+if(__is_long())
+__alloc_traits::deallocate(__alloc(), __get_long_pointer(), capacity() + 1);
+} 
+
+_LIBCPP_INLINE_VISIBILITY
 void __copy_assign_alloc(const basic_string& __str, true_type)
 {
 if (__alloc() == __str.__alloc())
 __alloc() = __str.__alloc();
 else
 {
 if (!__str.__is_long())
 {
-clear();
-shrink_to_fit();
+__clear_and_shrink();
 __alloc() = __str.__alloc();
 }
 else
 {
 allocator_type __a = __str.__alloc();
 pointer __p = __alloc_traits::allocate(__a, __str.__get_long_cap());
-clear();
-shrink_to_fit();
+__clear_and_shrink();
 __alloc() = _VSTD::move(__a);
 __set_long_pointer(__p);
 __set_long_cap(__str.__get_long_cap());
@@ -2102,8 +2108,7 @@
 _NOEXCEPT_(is_nothrow_move_assignable::value)
 #endif
 {
-clear();
-shrink_to_fit();
+__clear_and_shrink();
 __r_.first() = __str.__r_.first();
 __move_assign_alloc(__str);
 __str.__zero();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[John Baldwin via Phabricator via cfe-commits]

bsdjhb added a comment.

In https://reviews.llvm.org/D39074#974913, @sdardis wrote:

> This was libunwind's test suite:
>
>   Compiled test failed unexpectedly!
>   
>   Testing Time: 0.53s
>   
>   Failing Tests (1):
>   libunwind :: libunwind_02.pass.cpp
>  
> Expected Passes: 3
> Unexpected Failures: 1 
>   
>   
>
> The hacky patch I used to test n32:
>
>   --- a/test/libunwind/test/config.py
>   +++ b/test/libunwind/test/config.py
>   @@ -48,6 +48,8 @@ class Configuration(LibcxxConfiguration):
># Stack unwinding tests need unwinding tables and these are not
># generated by default on all Targets.
>self.cxx.compile_flags += ['-funwind-tables']
>   +self.cxx.compile_flags += ['-mabi=n33']
>   +self.cxx.link_flags += ['-mabi=n32']
>if not self.get_lit_bool('enable_threads', True):
>self.cxx.compile_flags += ['-D_LIBUNWIND_HAS_NO_THREADS']
>self.config.available_features.add('libunwind-no-threads')
>   
>   


Just to be sure, is that '-mabi=n33' in the compile flags a copy and paste typo 
in the diff or do you have it locally in the real change as well?


https://reviews.llvm.org/D39074



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 129690.
JonasToth marked 3 inline comments as done.
JonasToth added a comment.

- address review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: don't use macros to declare constants
+
+#define PROBLEMATIC_FUNCTION(x,y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: don't use macros to simulate functions
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: don't use macros to simulate variadic templates
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -54,6 +54,7 @@
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
cppcoreguidelines-pro-bounds-array-to-pointer-decay
Index: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - cppcoreguidelines-macro-usage
+
+cppcoreguidelines-macro-usage
+=
+
+Find macro usage that is considered problematic because better language
+constructs exist for the task.
+The relevant sections in the C++ Coreguidelines are 
+`Enum.1 `_,
+`ES.30 `_,
+`ES.31 `_ and
+`ES.33 `_.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,13 +57,20 @@
 Improvements to clang-tidy
 --
 
+- New `cppcoreguidelines-macro-usage
+  `_ check
+
+  Find macro usage that is considered problematic because better language
+  constructs exist for the task.
+
 - New `fuchsia-statically-constructed-objects
   `_ check
 
   Warns if global, non-trivial objects with static storage are constructed, unless the 
   object is statically initialized with a ``constexpr`` constructor or has no 
   explicit constructor.
 
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/cppcoreguidelines/MacroUsageCheck.h
===
--- /dev/null
+++ clang-tidy/cppcoreguidelines/MacroUsageCheck.h
@@ -0,0 +1,38 @@
+//===--- MacroUsageCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MACROUSAGECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MACROUSAGECHECK_H
+
+#include "../ClangTidy.h"
+#include "clang/Lex/Preprocessor.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+enum class MacroUsageMode { Constant, Function, Variadic };
+/// Find macro usage that is considered problematic because better language
+/// constructs exist for the task.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-macr

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 129691.
JonasToth added a comment.

- minor issues fixed


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: don't use macros to declare constants
+
+#define PROBLEMATIC_FUNCTION(x,y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: don't use macros to simulate functions
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: don't use macros to simulate variadic templates
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -54,6 +54,7 @@
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
cppcoreguidelines-pro-bounds-array-to-pointer-decay
Index: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - cppcoreguidelines-macro-usage
+
+cppcoreguidelines-macro-usage
+=
+
+Find macro usage that is considered problematic because better language
+constructs exist for the task.
+The relevant sections in the C++ Coreguidelines are 
+`Enum.1 `_,
+`ES.30 `_,
+`ES.31 `_ and
+`ES.33 `_.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,12 @@
 Improvements to clang-tidy
 --
 
+- New `cppcoreguidelines-macro-usage
+  `_ check
+
+  Find macro usage that is considered problematic because better language
+  constructs exist for the task.
+
 - New `fuchsia-statically-constructed-objects
   `_ check
 
Index: clang-tidy/cppcoreguidelines/MacroUsageCheck.h
===
--- /dev/null
+++ clang-tidy/cppcoreguidelines/MacroUsageCheck.h
@@ -0,0 +1,38 @@
+//===--- MacroUsageCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MACROUSAGECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MACROUSAGECHECK_H
+
+#include "../ClangTidy.h"
+#include "clang/Lex/Preprocessor.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+enum class MacroUsageMode { Constant, Function, Variadic };
+/// Find macro usage that is considered problematic because better language
+/// constructs exist for the task.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-macro-usage.html
+class MacroUsageCheck : public ClangTidyCheck {
+public:
+  MacroUsageCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void warnMacro(const MacroDirective *MD);
+};
+
+} // namespace cppc

[PATCH] D41815: [clang-tidy] implement check for goto

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst:6-7
+
+The usage of ``goto`` has been discouraged for a long time and is diagnosed
+with this check.
+

JonasToth wrote:
> aaron.ballman wrote:
> > This doesn't really help the user understand what's bad about goto or why 
> > it should be diagnosed. You should expound a bit here.
> Is it better now?
Much better now! Thank you for fix!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41815



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

[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 129693.
JonasToth added a comment.

rebase to 7.0.0


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40854

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.cpp
  clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-mixed-int-arithmetic.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp

Index: test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-mixed-int-arithmetic %t
+
+enum UnsignedEnum : unsigned char {
+  UEnum1,
+  UEnum2
+};
+
+enum SignedEnum : signed char {
+  SEnum1,
+  SEnum2
+};
+
+unsigned char returnUnsignedCharacter() { return 42; }
+unsigned returnUnsignedNumber() { return 42u; }
+long returnBigNumber() { return 42; }
+float unrelatedThing() { return 42.f; }
+SignedEnum returnSignedEnum() { return SEnum1; }
+UnsignedEnum returnUnsignedEnum() { return UEnum1; }
+
+void mixed_binary() {
+  unsigned int UInt1 = 42;
+  signed int SInt1 = 42;
+  UnsignedEnum UE1 = UEnum1;
+  SignedEnum SE1 = SEnum1;
+  float UnrelatedFloat = 42.f;
+
+  // Test traditional integer types.
+  auto R1 = UInt1 + SInt1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:21: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:13: note: unsigned operand
+
+  int R2 = UInt1 - SInt1;
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:20: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:12: note: unsigned operand
+
+  unsigned int R3 = UInt1 * SInt1;
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:29: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:21: note: unsigned operand
+
+  unsigned int R4 = UInt1 / returnBigNumber();
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:29: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:21: note: unsigned operand
+
+  char R5 = returnUnsignedCharacter() + SInt1;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:41: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:13: note: unsigned operand
+
+  auto R6 = SInt1 - 10u;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:13: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:21: note: unsigned operand
+
+  auto R7 = UInt1 * 10;
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:21: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:13: note: unsigned operand
+
+  auto R8 = 10u / returnBigNumber();
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:19: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:13: note: unsigned operand
+
+  auto R9 = 10 + returnUnsignedCharacter();
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:13: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:18: note: unsigned operand
+
+  // Test enum types.
+  char R10 = returnUnsignedEnum() - SInt1;
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:37: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]:14: note: unsigned operand
+
+  unsigned char R11 = returnSignedEnum() * UInt1;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: mixed signed and unsigned arithmetic; prefer signed integers and use unsigned types only for modulo arithmetic
+  // CHECK-MESSAGES: [[@LINE-2]]:23: note: signed operand
+  // CHECK-MESSAGES: [[@LINE-3]]

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

I just checked both my qemu copy and  on my mips64 machine and it seems to be a 
a copy / paste error. Reposting here directly from my machines:

MIPS64:

  diff --git a/test/libunwind/test/config.py b/test/libunwind/test/config.py
  index 2a0c828..a8952c3 100644
  --- a/test/libunwind/test/config.py
  +++ b/test/libunwind/test/config.py
  @@ -48,6 +48,8 @@ class Configuration(LibcxxConfiguration):
   # Stack unwinding tests need unwinding tables and these are not
   # generated by default on all Targets.
   self.cxx.compile_flags += ['-funwind-tables']
  +   self.cxx.compile_flags += ['-mabi=n32']
  +self.cxx.link_flags += ['-mabi=n32']
   if not self.get_lit_bool('enable_threads', True):
   self.cxx.compile_flags += ['-D_LIBUNWIND_HAS_NO_THREADS']
   self.config.available_features.add('libunwind-no-threads')

X86_64:

  diff --git a/test/libunwind/test/config.py b/test/libunwind/test/config.py
  index 2a0c828..f1953e2 100644
  --- a/test/libunwind/test/config.py
  +++ b/test/libunwind/test/config.py
  @@ -48,6 +48,8 @@ class Configuration(LibcxxConfiguration):
   # Stack unwinding tests need unwinding tables and these are not
   # generated by default on all Targets.
   self.cxx.compile_flags += ['-funwind-tables']
  +   self.cxx.compile_flags += ['-mabi=n32']
  +self.cxx.link_flags += ['-mabi=n32', 
'/home/sdardis/mips-mti-linux-gnu/2016.05-06/bin/../lib/gcc/mips-mti-linux-gnu/4.9.2/../../../../mips-mti-linux-gnu/lib/mips-r2-hard/lib32/libgcc_s.so.1']
   if not self.get_lit_bool('enable_threads', True):
   self.cxx.compile_flags += ['-D_LIBUNWIND_HAS_NO_THREADS']
   self.config.available_features.add('libunwind-no-threads')

I corrected whitespace nit before I posted the comment.


https://reviews.llvm.org/D39074



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[John McCall via Phabricator via cfe-commits]

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

Thanks.  LGTM, but you should wait for Eli's sign-off, too.


https://reviews.llvm.org/D40023



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

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

[Vedant Kumar via Phabricator via cfe-commits]

vsk added reviewers: EricWF, mclow.lists, hiraditya.
vsk added a comment.

Adding some folks who may be interested.


https://reviews.llvm.org/D41976



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

[PATCH] D40023: [RISCV] Implement ABI lowering

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.

LGTM

Not sure if anyone's mentioned it yet, but there's a C ABI testing tool in 
clang/utils/ABITest/ which you'll probably want to try at some point.




Comment at: lib/CodeGen/TargetInfo.cpp:8913
+  }
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+}

asb wrote:
> efriedma wrote:
> > The spec says "Aggregates larger than 2✕XLEN bits are passed by reference 
> > and are replaced in the argument list with the address".  That's not byval.
> The LLVM backend lowers byval in that way. Given that at this point we have a 
> trivial data type (plain struct or similar) which is copied-by-value by C 
> semantics, the only difference is whether the generated IR indicates implicit 
> copying with 'byval' or relies on the caller explicitly doing the copy. For 
> some reason I thought 'byval' was preferred, but looking again it seems 
> uncommon to do it this way. I've changed it to false - thanks for spotting 
> this.
"byval" generally means the value is memcpy'ed into the argument list (so there 
is no pointer in the argument list). This is useful for handling C calling 
conventions which allow excessively large structs to be passed in the argument 
list, so the backend can emit a memcpy rather than expanding the operation into 
straight-line code.  The RISCV backend handling of byval is wrong, in the sense 
that it isn't consistent with what any other backend does.

This isn't relevant to the RISC-V C calling convention, but you should probably 
fix the backend at some point to avoid future confusion.


https://reviews.llvm.org/D40023



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