[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe created this revision.
jgorbe added reviewers: christylee, rsmith, aaron.ballman.

r342177  introduced a hint in cases where an 
#included file is not found. It tries to find a suggestion by removing leading 
or trailing non-alphanumeric characters and checking if a matching file exists, 
then it reports an error like:

  include-likely-typo.c:3:10: error: '' file not 
found, did you mean 'empty_file_to_include.h'?
  #include "" // expected-error 
{{'' file not found, did you mean 
'empty_file_to_include.h'?}}
   ^~~
   "empty_file_to_include.h"
  1 error generated.

However, if a hint is **not** found, the error message will show only the 
trimmed name we use to look for a hint, so:

  #include "/non_existing_file."

will result in:

  include-leading-nonalpha-no-suggest.c:3:10: fatal error: 
'non_existing_file_to_include.h' file not found
  #include "/non_existing_file_to_include.h" // expected-error 
{{'/non_existing_file_to_include.h' file not found}}
   ^~~~
  1 error generated.

where the name reported after `"fatal error:"` doesn't match what the user 
wrote.

This change reports the original file name instead of the trimmed one when a 
suggestion is not found.


Repository:
  rC Clang

https://reviews.llvm.org/D52280

Files:
  lib/Lex/PPDirectives.cpp
  test/Preprocessor/include-leading-nonalpha-no-suggest.c
  test/Preprocessor/include-leading-nonalpha-suggest.c


Index: test/Preprocessor/include-leading-nonalpha-suggest.c
===
--- /dev/null
+++ test/Preprocessor/include-leading-nonalpha-suggest.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/empty_file_to_include.h" // expected-error 
{{'/empty_file_to_include.h' file not found, did you mean 
'empty_file_to_include.h'?}}
Index: test/Preprocessor/include-leading-nonalpha-no-suggest.c
===
--- /dev/null
+++ test/Preprocessor/include-leading-nonalpha-no-suggest.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/non_existing_file_to_include.h" // expected-error 
{{'/non_existing_file_to_include.h' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }


Index: test/Preprocessor/include-leading-nonalpha-suggest.c
===
--- /dev/null
+++ test/Preprocessor/include-leading-nonalpha-suggest.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/empty_file_to_include.h" // expected-error {{'/empty_file_to_include.h' file not found, did you mean 'empty_file_to_include.h'?}}
Index: test/Preprocessor/include-leading-nonalpha-no-suggest.c
===
--- /dev/null
+++ test/Preprocessor/include-leading-nonalpha-no-suggest.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/non_existing_file_to_include.h" // expected-error {{'/non_existing_file_to_include.h' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe added a comment.

Can't comment much on the patch itself (I'm still not very familiar with the 
codebase, I'll leave that to the other reviewers), but thanks a lot for 
responding so quickly! :)


https://reviews.llvm.org/D46135



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

[PATCH] D30963: Fix crash when an 'import a module' TypoCorrection has its CorrectionDecls replaced by visible decls.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe added inline comments.



Comment at: lib/Sema/SemaLookup.cpp:3792-3793
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
   if (TC.begin() == TC.end())
 return;
 

rsmith wrote:
> jgorbe wrote:
> > rsmith wrote:
> > > Do we need to clear the flag on this path too? (This might happen if the 
> > > old correction required an import and after clearing we set this up as a 
> > > keyword correction.)
> > If this might happen, then yes. I have updated the patch to clear the flag 
> > here as well but, on second thought, is this path necessary at all? It 
> > seems to me that 'no declarations' is a special case of 'all declarations 
> > are visible', which we check right after this.
> I agree, this check is redundant. Just go ahead and remove it :)
Done! :D

I don't have commit access yet, can you please commit it?


https://reviews.llvm.org/D30963



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

[PATCH] D30963: Fix crash when an 'import a module' TypoCorrection has its CorrectionDecls replaced by visible decls.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe updated this revision to Diff 101437.
jgorbe added a comment.
Herald added a subscriber: sanjoy.

Removed unneeded branch.


https://reviews.llvm.org/D30963

Files:
  lib/Sema/SemaLookup.cpp
  test/Modules/Inputs/crash-typo-correction-visibility/module.h
  test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
  test/Modules/crash-typo-correction-visibility.cpp


Index: test/Modules/crash-typo-correction-visibility.cpp
===
--- /dev/null
+++ test/Modules/crash-typo-correction-visibility.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=module -o 
%T/module.pcm -emit-module 
%S/Inputs/crash-typo-correction-visibility/module.modulemap
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%T/module.pcm %s 
-verify
+
+struct S {
+  int member; // expected-note {{declared here}}
+};
+
+int f(...);
+
+int b = sizeof(f(member)); // expected-error {{undeclared identifier 'member'}}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
@@ -0,0 +1,3 @@
+module "module" {
+  header "module.h"
+}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.h
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.h
@@ -0,0 +1 @@
+struct member;
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3746,20 +3746,19 @@
   bool FindHidden);
 
 /// \brief Check whether the declarations found for a typo correction are
-/// visible, and if none of them are, convert the correction to an 'import
-/// a module' correction.
+/// visible. Set the correction's RequiresImport flag to true if none of the
+/// declarations are visible, false otherwise.
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
-  if (TC.begin() == TC.end())
-return;
-
   TypoCorrection::decl_iterator DI = TC.begin(), DE = TC.end();
 
   for (/**/; DI != DE; ++DI)
 if (!LookupResult::isVisible(SemaRef, *DI))
   break;
-  // Nothing to do if all decls are visible.
-  if (DI == DE)
+  // No filtering needed if all decls are visible.
+  if (DI == DE) {
+TC.setRequiresImport(false);
 return;
+  }
 
   llvm::SmallVector NewDecls(TC.begin(), DI);
   bool AnyVisibleDecls = !NewDecls.empty();


Index: test/Modules/crash-typo-correction-visibility.cpp
===
--- /dev/null
+++ test/Modules/crash-typo-correction-visibility.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=module -o %T/module.pcm -emit-module %S/Inputs/crash-typo-correction-visibility/module.modulemap
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%T/module.pcm %s -verify
+
+struct S {
+  int member; // expected-note {{declared here}}
+};
+
+int f(...);
+
+int b = sizeof(f(member)); // expected-error {{undeclared identifier 'member'}}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
@@ -0,0 +1,3 @@
+module "module" {
+  header "module.h"
+}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.h
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.h
@@ -0,0 +1 @@
+struct member;
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3746,20 +3746,19 @@
   bool FindHidden);
 
 /// \brief Check whether the declarations found for a typo correction are
-/// visible, and if none of them are, convert the correction to an 'import
-/// a module' correction.
+/// visible. Set the correction's RequiresImport flag to true if none of the
+/// declarations are visible, false otherwise.
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
-  if (TC.begin() == TC.end())
-return;
-
   TypoCorrection::decl_iterator DI = TC.begin(), DE = TC.end();
 
   for (/**/; DI != DE; ++DI)
 if (!LookupResult::isVisible(SemaRef, *DI))
   break;
-  // Nothing to do if all decls are visible.
-  if (DI == DE)
+  // No filtering needed if all decls are visible.
+  if (DI == DE) {
+TC.setRequiresImport(false);
 return;
+  }
 
   llvm::SmallVector NewDecls(TC.begin(), DI);
   bool AnyVisibleDecls = !NewDecls.empty();
___
cfe-commits mailing list
cfe

[PATCH] D30963: Fix crash when an 'import a module' TypoCorrection has its CorrectionDecls replaced by visible decls.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe added inline comments.



Comment at: lib/Sema/SemaLookup.cpp:3792-3793
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
   if (TC.begin() == TC.end())
 return;
 

rsmith wrote:
> Do we need to clear the flag on this path too? (This might happen if the old 
> correction required an import and after clearing we set this up as a keyword 
> correction.)
If this might happen, then yes. I have updated the patch to clear the flag here 
as well but, on second thought, is this path necessary at all? It seems to me 
that 'no declarations' is a special case of 'all declarations are visible', 
which we check right after this.


https://reviews.llvm.org/D30963



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

[PATCH] D30963: Fix crash when an 'import a module' TypoCorrection has its CorrectionDecls replaced by visible decls.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe updated this revision to Diff 101302.
jgorbe added a comment.

Also clear the 'requires import' flag when the TypoCorrection has no decls at 
all.


https://reviews.llvm.org/D30963

Files:
  lib/Sema/SemaLookup.cpp
  test/Modules/Inputs/crash-typo-correction-visibility/module.h
  test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
  test/Modules/crash-typo-correction-visibility.cpp


Index: test/Modules/crash-typo-correction-visibility.cpp
===
--- /dev/null
+++ test/Modules/crash-typo-correction-visibility.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=module -o 
%T/module.pcm -emit-module 
%S/Inputs/crash-typo-correction-visibility/module.modulemap
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%T/module.pcm %s 
-verify
+
+struct S {
+  int member; // expected-note {{declared here}}
+};
+
+int f(...);
+
+int b = sizeof(f(member)); // expected-error {{undeclared identifier 'member'}}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
@@ -0,0 +1,3 @@
+module "module" {
+  header "module.h"
+}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.h
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.h
@@ -0,0 +1 @@
+struct member;
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3746,20 +3746,24 @@
   bool FindHidden);
 
 /// \brief Check whether the declarations found for a typo correction are
-/// visible, and if none of them are, convert the correction to an 'import
-/// a module' correction.
+/// visible. Set the correction's RequiresImport flag to true if none of the
+/// declarations are visible, false otherwise.
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
-  if (TC.begin() == TC.end())
+  if (TC.begin() == TC.end()) {
+TC.setRequiresImport(false);
 return;
+  }
 
   TypoCorrection::decl_iterator DI = TC.begin(), DE = TC.end();
 
   for (/**/; DI != DE; ++DI)
 if (!LookupResult::isVisible(SemaRef, *DI))
   break;
-  // Nothing to do if all decls are visible.
-  if (DI == DE)
+  // No filtering needed if all decls are visible.
+  if (DI == DE) {
+TC.setRequiresImport(false);
 return;
+  }
 
   llvm::SmallVector NewDecls(TC.begin(), DI);
   bool AnyVisibleDecls = !NewDecls.empty();


Index: test/Modules/crash-typo-correction-visibility.cpp
===
--- /dev/null
+++ test/Modules/crash-typo-correction-visibility.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=module -o %T/module.pcm -emit-module %S/Inputs/crash-typo-correction-visibility/module.modulemap
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%T/module.pcm %s -verify
+
+struct S {
+  int member; // expected-note {{declared here}}
+};
+
+int f(...);
+
+int b = sizeof(f(member)); // expected-error {{undeclared identifier 'member'}}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
@@ -0,0 +1,3 @@
+module "module" {
+  header "module.h"
+}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.h
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.h
@@ -0,0 +1 @@
+struct member;
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3746,20 +3746,24 @@
   bool FindHidden);
 
 /// \brief Check whether the declarations found for a typo correction are
-/// visible, and if none of them are, convert the correction to an 'import
-/// a module' correction.
+/// visible. Set the correction's RequiresImport flag to true if none of the
+/// declarations are visible, false otherwise.
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
-  if (TC.begin() == TC.end())
+  if (TC.begin() == TC.end()) {
+TC.setRequiresImport(false);
 return;
+  }
 
   TypoCorrection::decl_iterator DI = TC.begin(), DE = TC.end();
 
   for (/**/; DI != DE; ++DI)
 if (!LookupResult::isVisible(SemaRef, *DI))
   break;
-  // Nothing to do if all decls are visible.
-  if (DI == DE)
+  // No filtering needed if all decls are visible.
+  if (DI == DE) {
+TC.setRequiresImport(false);
 return;
+  }
 

[PATCH] D30963: Fix crash when an 'import a module' TypoCorrection has its CorrectionDecls replaced by visible decls.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe added a comment.

Ping?


https://reviews.llvm.org/D30963



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

[PATCH] D33108: Generate extra .ll files before/after optimization when using -save-temps.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe added inline comments.



Comment at: lib/Driver/Driver.cpp:2603-2614
+  // lipo-able.
+  if (!MultiArchUniversalBuild) {
+if (isSaveTempsEnabled() && Phase == phases::Compile) {
+  Actions.push_back(
+  C.MakeAction(Current, types::TY_LLVM_IR));
+}
+if (isSaveTempsEnabled() && Phase == phases::Backend) {

tra wrote:
> Can this be moved below addHostDependenceToDeviceActions() on line 2626?
> See my comment in cuda-options.cu below for the reasons why it may be 
> necessary.
I have tried but it didn't cause the test to go back to the previous ordering 
where all device-side actions are executed before all host-side actions.

I have noticed that, before applying my patch, the action graph produced by 
clang with -ccc-print-phases doesn't seem to introduce any explicit dependency 
to guarantee that device-side actions are executed before host-side actions:

0: input, "cuda-options.cu", cuda, (host-cuda)
1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
2: compiler, {1}, ir, (host-cuda)
3: input, "cuda-options.cu", cuda, (device-cuda, sm_20)
4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_20)
5: compiler, {4}, ir, (device-cuda, sm_20)
6: backend, {5}, assembler, (device-cuda, sm_20)
7: assembler, {6}, object, (device-cuda, sm_20)
8: offload, "device-cuda (nvptx64-nvidia-cuda:sm_20)" {7}, object
9: offload, "device-cuda (nvptx64-nvidia-cuda:sm_20)" {6}, assembler
10: linker, {8, 9}, cuda-fatbin, (device-cuda)
11: offload, "host-cuda (x86_64--linux-gnu)" {2}, "device-cuda 
(nvptx64-nvidia-cuda)" {10}, ir
12: backend, {11}, assembler, (host-cuda)
13: assembler, {12}, object, (host-cuda)

I'm trying to figure out now if there's something else that enforces that 
restriction, or the current compilation order being the right one is a happy 
coincidence that my patch happened to disturb. I'm new to the project, so I'm 
working under the assumption that I'm missing something, any hints will be 
appreciated :)


https://reviews.llvm.org/D33108



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

[PATCH] D33108: Generate extra .ll files before/after optimization when using -save-temps.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe added inline comments.



Comment at: lib/Driver/Driver.cpp:2600-2603
+  // When saving temps, add extra actions to write unoptimized and 
optimized
+  // IR besides the normal bitcode outputs if possible. This is not 
possible
+  // for multi-arch builds because in these builds we check that we can 
lipo
+  // all action outputs.

tra wrote:
> I'm not sure I understand why unoptimized IR can't be written out for 
> multi-arch builds like CUDA. Could you elaborate?
The error I was trying to work around here is [[ 
https://reviews.llvm.org/diffusion/L/browse/cfe/trunk/lib/Driver/Driver.cpp;304376$1407
 | this one ]].

It's not really specific to all multi-arch builds, but to Mach-O multi-arch 
builds. For those, BuildUniversalActions is called, which checks that all 
actions produce outputs with lipo-able types (see also the types::canLipoType() 
function) or errors out otherwise. [[ 
http://lists.llvm.org/pipermail/cfe-dev/2017-May/053811.html | I'm not sure the 
check is still necessary ]], but I got no answers when I asked about it in 
cfe-dev, so I decided to just not do it for that kind of builds.

I have updated the patch to make it clear that we only skip generating these IR 
files in that particular case, not every multi-arch build. Thanks for pointing 
this out!


https://reviews.llvm.org/D33108



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

[PATCH] D33108: Generate extra .ll files before/after optimization when using -save-temps.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe updated this revision to Diff 100954.
jgorbe added a comment.

Clarify that we only skip saving IR for multiarch Mach-O universal builds, not 
other multi-arch builds like CUDA.


https://reviews.llvm.org/D33108

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/cuda-options.cu
  test/Driver/save-temps.c

Index: test/Driver/save-temps.c
===
--- test/Driver/save-temps.c
+++ test/Driver/save-temps.c
@@ -1,9 +1,11 @@
 // RUN: %clang -target x86_64-apple-darwin -save-temps -arch x86_64 %s -### 2>&1 \
 // RUN:   | FileCheck %s
 // CHECK: "-o" "save-temps.i"
+// CHECK: "-o" "save-temps.unoptimized.ll"
 // CHECK: "-emit-llvm-uselists"
 // CHECK: "-disable-llvm-passes"
 // CHECK: "-o" "save-temps.bc"
+// CHECK: "-o" "save-temps.ll"
 // CHECK: "-o" "save-temps.s"
 // CHECK: "-o" "save-temps.o"
 // CHECK: "-o" "a.out"
Index: test/Driver/cuda-options.cu
===
--- test/Driver/cuda-options.cu
+++ test/Driver/cuda-options.cu
@@ -151,6 +151,12 @@
 // NOARCH-SM35-NOT: "-cc1"{{.*}}"-target-cpu" "sm_35"
 // ARCHALLERROR: error: Unsupported CUDA gpu architecture: all
 
+// Match host-side preprocessor job with -save-temps.
+// HOST-SAVE: "-cc1" "-triple" "x86_64--linux-gnu"
+// HOST-SAVE-SAME: "-aux-triple" "nvptx64-nvidia-cuda"
+// HOST-SAVE-NOT: "-fcuda-is-device"
+// HOST-SAVE-SAME: "-x" "cuda"
+
 // Match device-side preprocessor and compiler phases with -save-temps.
 // DEVICE-SAVE: "-cc1" "-triple" "nvptx64-nvidia-cuda"
 // DEVICE-SAVE-SAME: "-aux-triple" "x86_64--linux-gnu"
@@ -194,12 +200,6 @@
 // INCLUDES-DEVICE-DAG: "--image=profile=sm_{{[0-9]+}},file=[[CUBINFILE]]"
 // INCLUDES-DEVICE-DAG: "--image=profile=compute_{{[0-9]+}},file=[[PTXFILE]]"
 
-// Match host-side preprocessor job with -save-temps.
-// HOST-SAVE: "-cc1" "-triple" "x86_64--linux-gnu"
-// HOST-SAVE-SAME: "-aux-triple" "nvptx64-nvidia-cuda"
-// HOST-SAVE-NOT: "-fcuda-is-device"
-// HOST-SAVE-SAME: "-x" "cuda"
-
 // Match host-side compilation.
 // HOST: "-cc1" "-triple" "x86_64--linux-gnu"
 // HOST-SAME: "-aux-triple" "nvptx64-nvidia-cuda"
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -707,7 +707,7 @@
   if (TC.getTriple().isOSBinFormatMachO())
 BuildUniversalActions(*C, C->getDefaultToolChain(), Inputs);
   else
-BuildActions(*C, C->getArgs(), Inputs, C->getActions());
+BuildActions(*C, C->getArgs(), Inputs, C->getActions(), false);
 
   if (CCCPrintPhases) {
 PrintActions(*C);
@@ -910,7 +910,7 @@
   if (TC.getTriple().isOSBinFormatMachO())
 BuildUniversalActions(C, TC, Inputs);
   else
-BuildActions(C, C.getArgs(), Inputs, C.getActions());
+BuildActions(C, C.getArgs(), Inputs, C.getActions(), false);
 
   BuildJobs(C);
 
@@ -1392,7 +1392,7 @@
 Archs.push_back(Args.MakeArgString(TC.getDefaultUniversalArchName()));
 
   ActionList SingleActions;
-  BuildActions(C, Args, BAInputs, SingleActions);
+  BuildActions(C, Args, BAInputs, SingleActions, Archs.size() > 1);
 
   // Add in arch bindings for every top level action, as well as lipo and
   // dsymutil steps if needed.
@@ -2389,7 +2389,8 @@
 } // anonymous namespace.
 
 void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
-  const InputList &Inputs, ActionList &Actions) const {
+  const InputList &Inputs, ActionList &Actions,
+  bool MultiArchUniversalBuild) const {
   llvm::PrettyStackTraceString CrashInfo("Building compilation actions");
 
   if (!SuppressMissingInputWarning && Inputs.empty()) {
@@ -2595,6 +2596,22 @@
 break;
   }
 
+  // When saving temps, add extra actions to write unoptimized and optimized
+  // IR besides the normal bitcode outputs if possible. This is not possible
+  // for Mach-O multi-arch universal builds because in these builds we check
+  // that we can lipo all action outputs, and types::TY_LLVM_IR is not
+  // lipo-able.
+  if (!MultiArchUniversalBuild) {
+if (isSaveTempsEnabled() && Phase == phases::Compile) {
+  Actions.push_back(
+  C.MakeAction(Current, types::TY_LLVM_IR));
+}
+if (isSaveTempsEnabled() && Phase == phases::Backend) {
+  Actions.push_back(
+  C.MakeAction(Current, types::TY_LLVM_IR));
+}
+  }
+
   // Otherwise construct the appropriate action.
   auto *NewCurrent = ConstructPhaseAction(C, Args, Phase, Current);
 
@@ -3544,6 +3561,11 @@
 if (!AtTopLevel && C.getArgs().hasArg(options::OPT_emit_llvm) &&
 JA.getType() == types::TY_LLVM_BC)
   Suffixed += ".tmp";
+// When using -save-temps, append a ".unoptimized" suffix so that the
+// optimized .ll file doesn't overwrite the unoptimized one.
+if (isSaveTempsEnabled() && JA.getType() =

[PATCH] D32573: [Driver] Rename GetNamedOutputPath() to addNamedOutputPath()

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe created this revision.

The current name is confusing: it looks like it should just compute the path 
and return it but it has the side effect of appending it to the compilation's 
list of result files. This change changes the verb in the method name from 
"get" to "add" to emphasize this side effect.


https://reviews.llvm.org/D32573

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -3308,7 +3308,7 @@
   UI.DependentToolChain->getTriple().normalize(),
   /*CreatePrefixForHost=*/true);
   auto CurI = InputInfo(
-  UA, GetNamedOutputPath(C, *UA, BaseInput, UI.DependentBoundArch,
+  UA, addNamedOutputPath(C, *UA, BaseInput, UI.DependentBoundArch,
  /*AtTopLevel=*/false, MultipleArchs,
  OffloadingPrefix),
   BaseInput);
@@ -3338,7 +3338,7 @@
 A->getOffloadingDeviceKind(), TC->getTriple().normalize(),
 /*CreatePrefixForHost=*/!!A->getOffloadingHostActiveKinds() &&
 !AtTopLevel);
-Result = InputInfo(A, GetNamedOutputPath(C, *JA, BaseInput, BoundArch,
+Result = InputInfo(A, addNamedOutputPath(C, *JA, BaseInput, BoundArch,
  AtTopLevel, MultipleArchs,
  OffloadingPrefix),
BaseInput);
@@ -3416,7 +3416,7 @@
   return Args.MakeArgString(Filename.c_str());
 }
 
-const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
+const char *Driver::addNamedOutputPath(Compilation &C, const JobAction &JA,
const char *BaseInput,
StringRef BoundArch, bool AtTopLevel,
bool MultipleArchs,
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -449,19 +449,20 @@
   /// Returns the default name for linked images (e.g., "a.out").
   const char *getDefaultImageName() const;
 
-  /// GetNamedOutputPath - Return the name to use for the output of
-  /// the action \p JA. The result is appended to the compilation's
-  /// list of temporary or result files, as appropriate.
+  /// Computes the name to use for the output of the action \p JA and appends 
it
+  /// to the compilation's list of temporary or result files, as appropriate.
   ///
   /// \param C - The compilation.
   /// \param JA - The action of interest.
   /// \param BaseInput - The original input file that this action was
   /// triggered by.
-  /// \param BoundArch - The bound architecture. 
+  /// \param BoundArch - The bound architecture.
   /// \param AtTopLevel - Whether this is a "top-level" action.
   /// \param MultipleArchs - Whether multiple -arch options were supplied.
   /// \param NormalizedTriple - The normalized triple of the relevant target.
-  const char *GetNamedOutputPath(Compilation &C, const JobAction &JA,
+  //
+  //  \return The name to use for the output of the action \p JA.
+  const char *addNamedOutputPath(Compilation &C, const JobAction &JA,
  const char *BaseInput, StringRef BoundArch,
  bool AtTopLevel, bool MultipleArchs,
  StringRef NormalizedTriple) const;


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -3308,7 +3308,7 @@
   UI.DependentToolChain->getTriple().normalize(),
   /*CreatePrefixForHost=*/true);
   auto CurI = InputInfo(
-  UA, GetNamedOutputPath(C, *UA, BaseInput, UI.DependentBoundArch,
+  UA, addNamedOutputPath(C, *UA, BaseInput, UI.DependentBoundArch,
  /*AtTopLevel=*/false, MultipleArchs,
  OffloadingPrefix),
   BaseInput);
@@ -3338,7 +3338,7 @@
 A->getOffloadingDeviceKind(), TC->getTriple().normalize(),
 /*CreatePrefixForHost=*/!!A->getOffloadingHostActiveKinds() &&
 !AtTopLevel);
-Result = InputInfo(A, GetNamedOutputPath(C, *JA, BaseInput, BoundArch,
+Result = InputInfo(A, addNamedOutputPath(C, *JA, BaseInput, BoundArch,
  AtTopLevel, MultipleArchs,
  OffloadingPrefix),
BaseInput);
@@ -3416,7 +3416,7 @@
   return Args.MakeArgString(Filename.c_str());
 }
 
-const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
+const char *Driver::addNamedOutputPath(Compilation &C, const JobAction &JA,
const char *BaseInput,

[PATCH] D30963: Fix crash when an 'import a module' TypoCorrection has its CorrectionDecls replaced by visible decls.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe created this revision.

The TypoCorrection::RequiresImport flag is managed by
checkCorrectionVisibility() in SemaLookup.cpp. Currently, when none of the
declarations in the typo correction are visible RequiresImport is set to true,
and if there are no declarations, it's implicitly set to false by assigning a
default-constructed TypoCorrection. If all declarations are visible, nothing is
done.

The current logic assumes a TypoCorrection starts as a 'normal' correction and
gets converted into an 'import a module' correction when suggested fixes are not
visible. This is not always the case. The crash in the included test case is
caused by performQualifiedLookups() copying a TypoCorrection with
RequiresImport == true, clearing its CorrectionDecls, then finding another
visible declaration in a lookup. This results in an 'import a module' correction
for a visible declaration.

The fix makes checkCorrectionVisibility() set RequiresImport in all cases, so
the relationship between RequiresImport and suggestion visibility always holds
after returning.


https://reviews.llvm.org/D30963

Files:
  lib/Sema/SemaLookup.cpp
  test/Modules/Inputs/crash-typo-correction-visibility/module.h
  test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
  test/Modules/crash-typo-correction-visibility.cpp


Index: test/Modules/crash-typo-correction-visibility.cpp
===
--- /dev/null
+++ test/Modules/crash-typo-correction-visibility.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=module -o 
%T/module.pcm -emit-module 
%S/Inputs/crash-typo-correction-visibility/module.modulemap
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%T/module.pcm %s 
-verify
+
+struct S {
+  int member; // expected-note {{declared here}}
+};
+
+int f(...);
+
+int b = sizeof(f(member)); // expected-error {{undeclared identifier 'member'}}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
@@ -0,0 +1,3 @@
+module "module" {
+  header "module.h"
+}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.h
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.h
@@ -0,0 +1 @@
+struct member;
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3786,8 +3786,8 @@
   bool FindHidden);
 
 /// \brief Check whether the declarations found for a typo correction are
-/// visible, and if none of them are, convert the correction to an 'import
-/// a module' correction.
+/// visible. Set the correction's RequiresImport flag to true if none of the
+/// declarations are visible, false otherwise.
 static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
   if (TC.begin() == TC.end())
 return;
@@ -3797,9 +3797,11 @@
   for (/**/; DI != DE; ++DI)
 if (!LookupResult::isVisible(SemaRef, *DI))
   break;
-  // Nothing to do if all decls are visible.
-  if (DI == DE)
+  // No filtering needed if all decls are visible.
+  if (DI == DE) {
+TC.setRequiresImport(false);
 return;
+  }
 
   llvm::SmallVector NewDecls(TC.begin(), DI);
   bool AnyVisibleDecls = !NewDecls.empty();


Index: test/Modules/crash-typo-correction-visibility.cpp
===
--- /dev/null
+++ test/Modules/crash-typo-correction-visibility.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=module -o %T/module.pcm -emit-module %S/Inputs/crash-typo-correction-visibility/module.modulemap
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%T/module.pcm %s -verify
+
+struct S {
+  int member; // expected-note {{declared here}}
+};
+
+int f(...);
+
+int b = sizeof(f(member)); // expected-error {{undeclared identifier 'member'}}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.modulemap
@@ -0,0 +1,3 @@
+module "module" {
+  header "module.h"
+}
Index: test/Modules/Inputs/crash-typo-correction-visibility/module.h
===
--- /dev/null
+++ test/Modules/Inputs/crash-typo-correction-visibility/module.h
@@ -0,0 +1 @@
+struct member;
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -3786,8 +3786,8 @@
   bool FindHidden);
 
 /// \brief Check whether the declarations found for a ty